Vladimir Prus wrote:
Robert Ramey wrote:
instantiated from this code of mine:
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.
The intention is to trap the saving of tracked non-const objects. This is to prevent users from doing something like For(... A a; ... ar << a; // will save the address of a - which is on the stack
If a is tracked here, instances of a after the first will be stored only as reference ids. When the data is restored, all the as will be the same. Not what the programmer intended - and a bear to find. This is really the save counterpart to the load situation which required the implementation of reset_object_address.
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 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.
Further, how often is it that non-pointer object is tracked? I think it's rare case, while saving a pointer is a common case, and making common case inconvenient for the sake of non-common case does not seem right.
It can happen more often than you might think. Its easy to serialize an object and sometime later serialize a pointer to the same object. In order to avoid creating duplicates all the instances of an object - not just the pointers, have to be tracked if a pointer to that class of object is used even once. (As an aside, the serialization libary detects the situation where an object is never serialized through a pointer and suppresses tracking in this case).
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. 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 } while the following would create an error that would go undetected. for(...{ const X x = *it; // create a copy of ar << 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.
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.
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.
(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.
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? 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.
As I said, if its in a lot of places one should think about this. If this is truely intolerable and you don't mind driving without seat belts, use the & operator instead.
This is inferiour solution, because operator<< is more logical.
They're both arbitrary.
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. Personally, I believe const-ness is under appreciated and is helpful in catching bugs at compile time. In the future as more mult-threading is used, I think it will be even more important. Much effort was invested in using const (and asserts) to detect bugs at compile time. I can't catch them all but I catch what I can and I believe that it has made the library much easier to use. Robert Ramey