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.
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. 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.
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 2. Why the "watch" command on gdb is not a reliable way to catch such bugs?
Remember that when a tracked object is saved more than once, only the first time is the data saved. If the object can be changed during serialization, we have a problem.
Having said that - the & operator doesn't do the const checking. Doing so inhibits its usage. Also, in spite of much effort, I was unable to make the const checking function to my taste when objects are wrapped in an nvp wrapper.
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?
Where 'finish' returns auto_ptr<Data>. It's looks like serialization checks if the serialized type is 'const' and if not, complains. Basically, it's some heuritic to prevent saving on-stack object (though I don't understand why it would work at al).
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());
This would "fix" it but might not be necessary. I envisioned that the operator << would usually be used in a function templateof the following signature
save(Archive & ar, const T &t ,...
so that normally the issue wouldn't arise.
In the above case, I'm saving a specific object with a specific type. Using any function template is not an option.
If it does arise, its sort a warning to really think about what the code is doing. If the return value of data_builde.finish() can be changed during the serialization process, one is going to have a problem.
How can it be changed? Due to bug in 'save' for by class type? How often it happens?
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?
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.
Then, I get ambiguity in iserializer.hpp, in this code:
#ifndef BOOST_NO_FUNCTION_TEMPLATE_ORDERING template<class Archive, class T> inline void load(Archive &ar, const serialization::nvp<T> &t){ load(ar, const_cast<serialization::nvp<T> &>(t)); }
For some reason, both boost::archive::load and some other 'load' in 'boost' namespace (part of earlier serialization lib) that I still use are considered overload candidates. Adding explicit boost::archive:: fixes this. See attached patch.
Then I get error at this code:
ar << boost::lexical_cast<std::string>(*this);
The error message is:
error: no match for 'operator<<' in 'ar << boost::lexical_cast(Source) [with Target = std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Source = lvk::nm_model::BlockFormula]()' /space/NM/boost/boost/archive/detail/interface_oarchive.hpp:75: error: candidates are: Archive&
boost::archive::detail::interface_oarchive<Archive>::operator<<(T&) [with T = std::basic_string<char, std::char_traits<char>, std::allocator<char> >, Archive = boost::archive::binary_oarchive] ...unrelated operator<< definitions snipped...
Apparently, passing rvalue to "T&" does not work. Yes another attached patch fixes this issue.
This is the same issue as before. Passing a non-const tracked type to the archive << operator.
Ok, 1. Why std::string is 'tracked type'? 2. How do you suggest me to fix the above? - Volodya