Primitive types #23
Comments
|
I also tried to call |
|
Making enum a single level should be no problem. As far as your first issue goes, I'm not sure I have a good way of solving it that doesn't complicate the API unnecessarily. You can accomplish what you want by moving the actual toString call up a level, e.g.: archive( cereal::make_nvp("some cool name", myObject.toString()) ); // saving
std::string stringrep; // load the string
archive( cereal::make_nvp("some cool name", stringrep) ); // NVP not strictly required here, but we'll soon support out of order loading
myObject = MyObject( stringrep ); // construct from string, or whateverThe way our serialization code works right now, by the time we've entered into the save function we've already opened up a node in the JSON and started an object or array. I don't like the idea of bringing in more Boost functionality here just to get rid of one word in the JSON. There are a few functions in the JSON (and likely in the XML) archives that are not intended for public use (they have to be coupled with other functions for setting up/tearing down state) and those will get changed to be private. |
|
Actually I just thought of how you can solve your issue quite easily, through the same mechanism I'll be using with the enum types. If you look into the JSON archive header, you'll notice that we have various prologue/epilogue functions that get called immediately before and immediately after the serialize function is called for a specific type. You can use these to control when the JSON archive creates/ends nodes and get the functionality you desire. It's a little more effort than us providing a macro, but the prologue/epilogue functions provide a lot more power than fixed functionality. |
|
Thanks for your answers. +1 in advance for putting in the std::is_enum or !std::is_enum in the appropriate enable_ifs for JSON and XML. (As I read from your second thoughts, you agree that the "one level up" approach is quite unsatisfactory, because it breaks encapsulation/unity etc. ) I was indeed playing around with those enable_if lines (tried the enum first). I still hold the opinion that you should offer a trait for this (e.g. 'is_cereal_primitive') that the user can set for his class if he needs/wants to and that your archive impls use in all the places you now use 'is_enum'. I also think you will someday need/want to use the same technique for 'is_bitwise_serializable' As I am into traits for no longer than a total of 3-4 hours I cannot figure that out atm. |
|
It's actually simpler than expected: I added the following lines to cereal.hpp and used cereal::is_primitive in the appropriate enable_if lines of the archive if I now put after my class definition, it serializes as expected. |
|
That's a fairly clean solution but I'm still not sure about introducing it into cereal, since it can be accomplished by just writing your own prologue/epilogue functions (which can be anywhere - these don't need to reside in json.hpp for example), which is what I would prefer to see someone do. So basically your solution minus the traits, which is more verbose in general but keeps the interface cleaner. It would look like, for your example: (in one of your headers, so long as it knew about JSONOutputArchive) void prologue( JSONOutputArchive & ar, testclass const & t )
{
// whatever is needed/not needed
}
void epilogue( JSONOutputArchive & ar, testclass const & t )
{
// whatever is needed/not needed
}You could then use this overload, which would be preferred over the generic template version, to control whether your class needed to do setup/teardown operations for the archive. I'm not sure if there is going to be a trivial way to make the various internal functionality of the JSON/XML archives private, since unfortunately a friend declaration like |
|
Valid solution, too, but... I think we are now entering the arena of clean code discussion, always leading to a fight :o) 1 2 3 4 5 |
|
The solution I like is to introduce another set of serialization functions, e.g. We would obviously add non-member versions, and a load_and_allocate version as well. |
|
Initially this was only going to make it in as a change to enums but the current plan is to do something more along your request for something like |
|
Nice to hear. 1 2 |
|
My second intention concerning if a class is flagged as 'primitive'. Maybe this could even happen automatically!? |
|
I do want it to happen automatically if possible, my only concern is if someone tries to serialize something with either an NVP or more than one thing within their serialization function for a class declared as "primitive", it will lead to undefined behavior that we won't catch at compile time. This leads to the potential need for specialized "minimal" serialization functions for this case. Use case from a user would be something like adding either serialize_minimal, load_minimal, (etc), which we would automatically detect and use for a text archive. Still haven't fully decided on the interface for all of this. |
|
Quick update on this - the current design, which you can follow here, is to add a new pair of serialization functions to cereal: load_primitive and save_primitive (both member and non-member versions). The interface will be something like struct A
{
template <class Archive>
std::string save_minimal( Archive & ar )
{ /* somehow bundle self up into a string, or any primitive type */ }
template <class Archive>
void load_minimal( Archive & ar, std::string const & data )
{ /* loaded data contained in data, type must match return type of save_minimal */ }
};You won't be able to mix these with other serialization types unless you tell cereal to explicitly use one type. You can pick the return type of save so that it outputs any primitive type or a string. It may be that we end up not passing the archive so you can't try and save or load extra stuff here, though we'll probably keep the template type to allow specialization. enums will default to a minimal numerical representation after all of this. |
|
Sounds reasonable, but a little confusing to me. Because we are talking about primitive types, supported by every archive, I don't think, template specialization will ever be needed or make sense. Possible issues (apart from templatization). Maybe you will be better off using just |
|
The problem of 'automatical primitive' saving/loading a class with only one entry could probably better be solved at runtime by just NOT inserting "value0:..." if only one entry is saved. Only if a second value is saved within the same context, the first entry is corrected to "value0:..." and every subesequent one is then named "valueN:...". Just a thought of a non-meta programmer... |
|
You are right about the enums, that slipped my mind. The simplest solution is just the one you mentioned earlier in this discussion by marking a class as primitive and not opening a node. However, without storing any state in the archive itself, there is nothing stopping the user from trampling over this promise to only serialize one thing. If we add state for this we add runtime overhead, which leads to our preference for a compile time solution. It also adds a requirement that anyone implementing a text archive needs to implement this excess state machinery, and altering parse trees could potentially be really annoying. I think the best way to go about this will be to add an internal use only specifier that acts just like the It will be impossible to have two of them with differing return types as signatures for functions depend only on name + parameters. Having a save with a different return value than the load accepts as a parameter will be a compile time error. If the user defines a standard save and a save_minimal, that will be an error just as if they defined a serialize and a save/load pair. You have to pick one type of serialization, which can be either in or out of the class: serialization function, load/save pair, minimal load/save pair. The only way to get around this single serialization function type is to use the specialization functionality in access.hpp (see bottom of http://uscilab.github.io/cereal/serialization_functions.html for an example). |
|
Your last 2 paragraphs sound very simple and clear. You have too keep it that way. My only concern is my following use-case (think of files in the >> 1GB range): Implementing this might well be in 'my responsibility'. But mentioning |
Re-made a repo based on a more recent version of develop. Syntax for save_minimal looking good so far.
If someone attempts to have both a versioned and non-versioned serialization function, they will get a compile time error. relates to improvements in #23
|
Opened a stack overflow post related to this issue: http://stackoverflow.com/questions/22464225/c-detecting-free-function-existence-with-explicit-parameters For non-member template <class Archive>
double save_minimal( MyType const & mt );
template <class Archive>
void load_minimal( MyType & mt, double const & value );The tricky part here is writing a trait that, knowing |
writing type traits is a wonderful battle against the compiler see #23
|
The current implementation of save/load minimal have signatures like (note the lack of actually taking the archive as a parameter): template <class Archive>
double save_minimal( MyType const & mt );
template <class Archive>
void load_minimal( MyType & mt, double const & value );The archive template is so that users can specialize on different archive types (e.g. only do minimal for JSON or something). We don't currently pass archive as an actual parameter since there is nothing the user could potentially be doing with it in this situation. This design is presenting problems with name resolution for various reasons. Essentially if we have a load_minimal (or save_minimal) non-member function whose type does not exist in the same namespace as the function itself, we can't resolve it. Normally types are defined in the same namespace as their serialization functions, but if any serialization function is written in the cereal namespace (such as the built in functions we provide), we'll run into name resolution errors. The easiest way I can think of solving this is to change the signatures to look like: template <class Archive>
double save_minimal( Archive const &, MyType const & mt );
template <class Archive>
void load_minimal( Archive const &, MyType & mt, double const & value );Note that there is still nothing a user can actually do with the archive in these situations since it will be passed by constant reference, which is different from every other serialization function, which accepts its archive by reference. This doesn't look as elegant but it is probably necessary. Any objections? This is probably the last roadblock in getting this feature finished. Ultimately things declared with minimal functions will just end up wrapping their types in something similar to NameValuePair, except it will be something like MinimalWrapper, which archives can detect and handle appropriately (e.g. not opening new nodes). |
|
Despite the good work you have put in, I feel we are somehow over-complicating a simple matter. The fact that there (would) exist two different ways to serialize is a big warning sign to me. |
|
As a much simpler alternative, I suggest the following:
I cannot assess all the implications of this right now, but it seems much simpler and natural to have the choice at this level. The advanced user (see my use case above) can also choose at this stage, whether he wants to save as a string (for text-archives) or as raw/optimized/compressed/whatever (for binary archives). The compile time checking now takes place one level "deeper", and should simplify a lot because checking for minimal vs. non-minimal templates drops out. |
|
I'd love to go with a simpler solution. Here are the potential issues I see with an
This is what led me towards the implementation I've been working on - it's more complicated to implement behind the scenes, but easier for an archive to deal with and probably much easier for a user to deal with. The only concern is the interface for the function - it will be different regardless, but the API I'm leaning towards is the one I mentioned earlier (with the const & archive). Here's what a use case would look like: struct MyType
{
// some class variables
template <class Archive>
std::string save_minimal( Archive const & ) // no need to use the archive yet, maybe in the future
{
return some_complicated_way_of_making_a_string();
}
template <class Archive>
void load_minimal( Archive const &, std::string const & data )
{
some_complicated_way_of_loading_from_a_string( data );
}
// compile time error if load_minimal's second parameter is not a constant
// reference to the return type of save_minimal
// compile time error if a user also defines a serialize, or load/save pair
// for the same archive type for the same data type (e.g. MyType)
// user can also define the above with version parameter or
// outside of a class just like the other serialization functions
};
int main()
{
{
cereal::JSONOutputArchive ar( std::cout );
MyType mt( some, type, of, constructor );
ar( mt ); // executes the save_minimal with the name being autogenerated
ar( CEREAL_NVP(mt) ); // provide some NVP
// note how the interface to the user is identical to the other types of serialization
// there is also no way for a user to insert more information than one single
// primitive (arithmetic or string)
}
} |
Still some kinks to work out in regards to trait checks on a non-member load_minimal that accepts everything as template parameters and does enable_if style checks
|
True, I missed your point 2 in my (incomplete) consideration. What to do if the user calls |
|
Your use case looks pretty simple. For my purposes, I would like to add a specialization for Binary Archives roughly like this: because I want the minimal (to/from string) functions to be applied by default, except for binary archives, where I want to use raw data without the string conversions. How would I do this correctly? Possible issues:
|
|
You'd do something like this: struct MyType
{
template <class Archive>
typename std::enable_if<std::is_same<Archive, cereal::BinaryInputArchive>::value ||
std::is_same<Archive, cereal::BinaryOutputArchive>::value, void>::type
serialize( Archive & ar )
{
// whatever
}
// or alternatively, use this macro already in traits:
template <class Archive>
CEREAL_ARCHIVE_RESTRICT(cereal::BinaryInputArchive, cereal::BinaryOutputArchive)
serialize( Archive & ar )
{
// whatever
}
template <class Archive>
typename std::enable_if<!std::is_same<Archive, cereal::BinaryOutputArchive>::value,
std::string>::type // replace string with whatever minimal type you need
save_minimal( Archive const & ) const
{
}
template <class Archive>
typename std::enable_if<!std::is_same<Archive, cereal::BinaryInputArchive>::value,
void>::type
load_minimal( Archive const &, std::string const & data )
{
}
};You have to use |
|
99% done with this, all that is left is making sure VS plays nicely and updating comments, then 1.0 is basically done. |
|
That's exciting! <3 This is really one of the best libraries I've come across. |
|
Finally closing this - looking good from the testing I've done, but juggling four compilers is getting annoying. In the future I think it is highly likely we will drop support for GCC 4.7.3. |
|
Thx for the hints. |
I am missing the equivalent of
BOOST_CLASS_IMPLEMENTATION(MyClass, primitive_type)
Example:
I have classes that already know how to convert to/from a string.
so I use
resulting in the JSON
but WHAT I WANT is just a single line
Remark: the same thing happens for ENUMs in your implementation.
They are always saved as a 2-level object, which seems unnecessary.
The text was updated successfully, but these errors were encountered: