USCiLab / cereal

A C++11 library for serialization
BSD 3-Clause "New" or "Revised" License
4.22k stars 758 forks source link

Traits seem to not support references and const-references. #171

Open dkorolev opened 9 years ago

dkorolev commented 9 years ago

Hi,

I just had to change

cereal::traits::is_output_serializable<T, cereal::JSONOutputArchive>::value;

into

cereal::traits::is_output_serializable<typename std::remove_const<typename std::remove_re ference<T>::type>::type, cereal::JSONOutputArchive>::value;

Should it be handled by Cereal internally?

Thanks, Dima

AzothAmmo commented 9 years ago

Can you give an example of the code that required this change to be made?

dkorolev commented 9 years ago

https://github.com/dkorolev/sandbox_cereal/blob/master/standalone/traits.cc

#include <iostream>

#include "../cereal/archives/json.hpp"

struct X {
  std::string s;
  template<class A> void serialize(A& ar) {
    ar(CEREAL_NVP(s));
  }
};

template<typename T> void foo(T&& x) {
  std::cout
    << std::boolalpha
    << cereal::traits::is_output_serializable<T, cereal::JSONOutputArchive>::value
    << std::endl;
}

int main() {
  X x;
  const X& y(x);
  foo(x);  // true
  foo(y);  // false
}

As long as `foo() does any compile-type dispatching into cerealizable/non-cerealizable, the code would result in a nontrivial issue to catch.

AzothAmmo commented 9 years ago

We made a design decision at the beginning of cereal not to support raw pointers or references since we did not want to deal with the overhead (in terms of runtime and code) of having to track ownership ourselves to prevent duplicate loads/saves. There's more on this on the main web page at the bottom.

I think at best we should supply a proper static assertion saying that references are not supported when someone attempts to serialize them - it is not our intention to allow a reference to be serializable.

dkorolev commented 9 years ago

Hi Shane,

Thank you for circling back!

For me, the issue is that cereal::traits::is_output_serializable<T, cereal::JSONOutputArchive>::value is true for non-cereal types.

My usecase is HTTP response generation from C++. The logic is simple: Send Cereaizable objects as JSON-s, strings as strings, fail on the rest. Cereal's traits turn out to not be useful since, for example, std::vector gets JSON-ified as well.

Is there a way to use Cereal's traits to only tell if this type specifically defines Cereal-friendly serialize() or save()?

Thanks, Dima

Thanks, Dima

http://dimakorolev.com/ | http://www.linkedin.com/in/dimakorolev | dmitry.korolev@gmail.com | +1 (312) 593 2783

On Sat, Feb 28, 2015 at 11:53 AM, Shane Grant notifications@github.com wrote:

We made a design decision at the beginning of cereal not to support raw pointers or references since we did not want to deal with the overhead (in terms of runtime and code) of having to track ownership ourselves to prevent duplicate loads/saves. There's more on this on the main web page http://uscilab.github.io/cereal/pointers.html at the bottom.

I think at best we should supply a proper static assertion saying that references are not supported when someone attempts to serialize them - it is not our intention to allow a reference to be serializable.

— Reply to this email directly or view it on GitHub https://github.com/USCiLab/cereal/issues/171#issuecomment-76542718.

AzothAmmo commented 9 years ago

The traits are actually already checking if something specifically provides a cereal friendly serialize/save/etc. I think the issue you might be having is that when you pull in the default support for vectors via <cereal/types/vector.hpp>, this defines support for all POD types used with vectors as well. Am I correct or is there a different issue?

dkorolev commented 9 years ago

Yes, I pull in support for vector-s.

Sorry for introducing the confusion: We are talking about two subject in parallel here, due to my mistake misreading your previous comment.

Subject 1: cereal::traits::is_output_serializable<> does not support const reference types. Subject 2: cereal::traits::is_output_serializable<> returns true for basic types like strings or vectors, while I was hoping it would only return true for user-defined types that define template<typename A> void serialize() or template<typename A> void save() const.

To me:

Thanks, Dima