Boost C++ Libraries: Ticket #3977: Compile error on make_nvp with reference data member

attachment set

kondo@… — Wed, 03 Mar 2010 22:38:33 GMT

attachment → ref_direct.cpp

an example program to reproduce problem

attachment set

kondo@… — Wed, 03 Mar 2010 22:40:21 GMT

attachment → nvp_const_ref.patch

patch file to resolve this problem

component changed; owner set

anonymous — Wed, 03 Mar 2010 22:41:03 GMT

owner set to Robert Ramey
component None → serialization

attachment set

kondo@… — Wed, 03 Mar 2010 22:48:37 GMT

attachment → ref_direct2.cpp

replace TAB to white space version of ref_direct.cpp

kondo@… — Wed, 03 Mar 2010 22:49:06 GMT

Sorry, In ref_direct.cpp, white spaces and TABs are mixed. I replace all TABs to white spaces in ref_direct2.cpp The program meaning is not changed.

status changed; resolution set

Robert Ramey — Thu, 04 Mar 2010 06:48:58 GMT

status new → closed
resolution → invalid

this is not an error.

It's telling you not to pass a non-const.

try fixing it with a const_cast instead.

Your fix will actually create an error in some instances. The test suite doesn't test this specifically.

status changed; resolution deleted

kondo@… — Thu, 04 Mar 2010 11:09:42 GMT

status closed → reopened
resolution invalid

Thank you for a quick answer.

It's telling you not to pass a non-const. try fixing it with a const_cast instead.

You're right.

I used the static_cast, and it works correctly.

  ar << boost::serialization::make_nvp("ref_", static_cast<MyInt * const &>(&p->ref_));

Your fix will actually create an error in some instances. The test suite doesn't test this specifically.

Oh, I overlooked the side effect.

My overload matches to temporary object that shouldn't match.

boost::serialization::make_nvp("MyInt", MyInt());

I still think that I can remove the cast from the client code without a side effect.

I improved my patch to match only object that should match.(nvp_const_ref_ptr_special.patch)

It matches only address of object, it no longer doesn't match temporary objects.

How do you think about it?

Of course I carried out all tests again.

And all of them succeeded.

attachment set

kondo@… — Thu, 04 Mar 2010 11:11:05 GMT

attachment → nvp_const_ref_ptr_special.patch

improved patch file

status changed; resolution set

Robert Ramey — Thu, 04 Mar 2010 19:15:33 GMT

status reopened → closed
resolution → wontfix

This looks much better.

But I have still have concerns about it.

The reason that we trap a attempt to serialize a non-const is described in the rationale section of the documentation. Your fix overrides that an makes it easier for a user to create an archive which will prove to be unloadable. The current situation requires that the user either change his ref to a const ref or use a const_cast - also he could suppress tracking for that type. So the current situation requires that the user react to a potentially sever problem and take some explicit action to get around it. This saves me uncountable hours of explaining to the user where his problem might be when he later tries to load a tracked item which has changed in the course of serialization. I realize that it's a pain in many cases. But I really can't spend this much time with users who have inadvertently created unloadable archives.

Robert Ramey

kondo@… — Mon, 08 Mar 2010 13:31:11 GMT

Now, I have understood your policy. Serializing the reference data member is asymmetric. And we should write such code. But my patch helps hide the such code.

Now, I think to prepare below cast in my code.

template <typename T>
T* const& reference_serialize_cast(T* const& t)
{
    return t;
}

    ar << boost::serialization::make_nvp("ref_", reference_serialize_cast(&p->ref_));

By the way,

The reason that we trap a attempt to serialize a non-const is described in the rationale section of the documentation

means http://www.boost.org/doc/libs/1_42_0/libs/serialization/doc/rationale.html ?

or you mean http://www.boost.org/doc/libs/1_42_0/libs/serialization/doc/serialization.html#references ?