Robert Ramey wrote:
ofstream ofs(av[2]); boost::archive::binary_oarchive oa(ofs); oa << *data_builder.finish();
This code should be considered erroneous. The documentation in 1.32 addressed this but unfortunately the enforcement was lost in the shuffle.
Excuse me, but I refuse to accept that this code is erroneous. As I've said, this is my existing design -- the 'finish' function returns non-const pointer. The fact that somewhere else, I serialize the result of 'finish' has nothing to do with 'finish' return type, and I should not be forced to change my existing interface just because I use serialization somewhere.
OK let me rephrase. This code conflicts with the description of the description of the functioning of the operator << as described in the documentation.
Can you give me specific doc URL? I can't find the relevant part in TOC.
So, you're saing that "const" == "no allocated on stack"? I don't see why this statement is true. I can just as well do this:
void foo(const A& a) { ar << a; }
and circumvent your protection.
The intention is that const indicates that the process of serialization will not change the object being serialized. Serializing and object that cannot easily be passed as a const is quite possibly an error. Of course, the contrary is not necessarily true. That isan object
What's "isan"?
can be passed as const referene and still be modified during the process of serialization. So this is not bullet proof - but I believe it is very helpful.
Let me draw an analogy: there's std::ostream. For years, we save objects to ostreams and we're not required to save "const" object. The fact that serialization acts in a different way is inconsistent and confusing. I think you'll agree that users should not read documentation just to save an object. And let me clarify again -- is this indended to stack only stack allocated objects?
Note that the documentation suggests that he above be reformulated as For(... ar << a[i]; //
Enforcing const-ness also has the effect of preventing serialization from altering the state of the object being serialized - another almost impossible to find bug.
Can you explain: 1. How this bug can happen
Its very easy to write for(...{ X x = *it; // create a copy of ar << x }
all the x's are at the same address so if they happen to be tracked because a pointer to some X in serialized somewhere in the program, then the subsequent copies would be supprsed by tracking.
So, you're saying that the behaviour of the above code will be different, depending on whether some pointer to X is saved somewhere else? This is very strange -- here, saving is not done via pointer, so I expect that no tracking is ever done. I'd argue this is a bug in serialization. The only problematic case is for(...{ X x = *it; // create a copy of ar << x } X* x = new X: ar << x; where address of newed 'x' is the same as address of saved 'x'. But this can never happen, because heap memory and stack memory are distinct.
With the above formulation this cannot happen. This error is detected a compile time - very convenient. Of course the following would work as well
for(...{ const & X x = *it; // save a const reference to the orgiinal ar << x }
Will that indeed save a const reference? How will you read const reference from an archive?
while the following would create an error that would go undetected.
for(...{ const X x = *it; // create a copy of ar << x }
Probably the right way to fix this is just don't track 'x' here. Again, no heap allocated object will have the same address as 'x'.
2. Why the "watch" command on gdb is not a reliable way to catch such bugs?
I'm not sure what the gdb watch command does but I'm sure it doesn't detect compile time errors.
It allows to find all places where a specific memory location is modified. So, if you have a bug where a value of an object is modified by serialization, it's easy to track this down to specific code line.
Also, I had to tweak a number of my tests and demos to make them work with this new rule. However, the tweaking was not difficult. If altering one's code to conform this rule isn't easy, one should make an effort to understand why. Its possible that a very subtle and hard to understand bug might be being introduced.
Are you saying that my code is buggy?
I'm saying you've passed up an opportunity to permit the compiler to flag someting that could be an error.
ofstream ofs(av[2]); boost::archive::binary_oarchive oa(ofs); oa << *data_builder.finish();
Looking at the above, I have to concede I never envisioned it being used this way. I would need more context to really comment intelligently on it. I will say that I alwas envisioned the interface as being used in a declarative style. That is, I envisioned the the serialization declarations ar & x or ar << x as a shorthand for "this member is persistent". I'm not saying that you're doing anything wrong, its just not what I expected.
What kind of context should I provide? This is top-level code, it reads some files, passes them to 'data_builder' and then calls 'finish' that builds the proper data which is then saved.
Where 'finish' returns auto_ptr<Data>. It's looks like serialization checks if the serialized type is 'const' and if not, complains.
correct
Basically, it's some heuritic to prevent saving on-stack object
That's one case I would like trap.
Any others?
(though I don't understand why it would work at al).
I don't understand this.
I find this a bit too much . I have no reason whatsoever to make 'finish' return auto_ptr<const Data>. And writing
oa << const_cast<const Data&>(*data_builder.finish());
When finish returns something that is not a const it suggests that its returning a pointer to a mutable object. The serialization library presumes that objects don't change in the course of serialization. So passing a non-const conflicts with one of the assumptions made in the implementation of the library.
Ehm... the standard way to solve this is to declare "operator<<" with the "const T&" type, just like iostreams do.
That is why the STATIC_ASSERT only trips when tracking is enabled for the corresponding datatype.
looks very strange. And modifying all places where I get this error is not nice too. So, can this static assert be removed?
Its there for a purpose. How many places do you get this error? Returning an auto_ptr to the top of the stack is interesting to me. doesn't that destroy the one in the original location?
No, returning auto_ptr from a function is one of its intended usages.
If its a temporary you could just as well return an auto_ptr to a const T. But really without more context its hard for me to commment.
1. I don't want to change my design due to serialization library. It's intrusive. 2. Inside 'finish', I need to work with auto_ptr<T> -- because I modify the data. And conversion from auto_ptr<T> to auto_ptr<const T> does not work for me on gcc.
Ok, 1. Why std::string is 'tracked type'?
so that storage space isn't wasted storing repeated strings
2. How do you suggest me to fix the above?
I did suggest using & instead. But I'm curious to see more context. I'm curious to see more.
Here's the enclosing method: template<class Archive> void save(Archive & ar, unsigned int version) const { // Serialization has problems with shared_ptr, so use strings. ar << boost::lexical_cast<std::string>(*this); } - Volodya