Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#4370 closed Bugs (wontfix)

[Serialization] map with private default constructor does not compile

Reported by: anonymous Owned by: Robert Ramey
Milestone: Boost 1.44.0 Component: serialization
Version: Boost 1.40.0 Severity: Problem
Keywords: Cc:

Description

I believe that collections_load_impl.hpp is somewhat incorrect in its implementation of loading for maps. The creation of the value_type of the map on the stack results in a call to the default std::pair constructor which will fail if the keys or the values do not have a public default constructor. Note that this problem is specific to maps and does not show in sequential containers.

In the case of nondefault constructible key/value the default constructor of the pair cannot be used.

Problem appears on Ubuntu with gcc 4.4.3 and the default (for Ubuntu) boost version 1.40. I do not have a compiled copy of boost 1.43, but it seems that the code is still the same. There was also no mention of the issue as being fixed in the recent changes to the Serialization library since 1.40.

A simple file to reproduce the problem is attached.

Attachments (2)

test.cpp (1.6 KB ) - added by anonymous 12 years ago.
test_g++.out (4.1 KB ) - added by anonymous 12 years ago.

Download all attachments as: .zip

Change History (12)

by anonymous, 12 years ago

Attachment: test.cpp added

comment:1 by Robert Ramey, 12 years ago

When I compile your test, I get a compilation error at the following statement

std::map<int,A>::value_type m1(1,A(10));

This has nothing to do with the serialization library. I don't see how I could possibly fix this from within the serialization library.

I believe your commented out fix is the correct one since the elements of A are in fact created by std::pair

I'm labeling this "wont fix" only because there is not "cant fix"

Robert Ramey

comment:2 by Robert Ramey, 12 years ago

Resolution: wontfix
Status: newclosed

comment:3 by anonymous, 12 years ago

Unfortunately I have to disagree and I would like to kindly ask that you take another look at this. Yes the compiler complains about std::pair construction, but not the one you referred to -- at least my compiler complains about the construction in /usr/include/boost/serialization/access.hpp:123 (please, take a look at the compiler output which is attached). I suggest that you comment out lines 60 and 65 (the ones with BOOST_SERIALIZATION_NVP(amap) ) and you will see that the line you referred to in your reply is fine because the constructor in line 54 is the one which takes the inputs as references; in fact the only solution I have been able to come up with is to modify collections_load_impl.hpp to use the constructor of std::pair which takes references as inputs as opposed to no inputs and calls the default constructors).

It is possible that I am misunderstanding something and in such case I would appreciate if you help me clear up my confusion (even though you certainly do not have to).

NK

by anonymous, 12 years ago

Attachment: test_g++.out added

comment:4 by anonymous, 12 years ago

Resolution: wontfix
Status: closedreopened

comment:5 by Robert Ramey, 12 years ago

I've double checked.

I do indeed get a compiler error on the following line

std::map<int,A>::value_type m1(1,A(10));

which I'm not really understanding as you point out shouldn't be instantitiating the default constructor. In any case, that's an MSVC and/or standard library issue.

I do see the code in collections_load_impl.hpp you refer to.

So ... what do you want me to do? Do you have a patch to submit?

Robert Ramey

in reply to:  5 comment:6 by anonymous, 12 years ago

I will try to put something together, but it will take me some time (as I have been using the default boost on Ubuntu) because I will need to compile my own local copy and familiarize myself with the inner workings of the serialization library. NK

Replying to ramey:

I've double checked.

I do indeed get a compiler error on the following line

std::map<int,A>::value_type m1(1,A(10));

which I'm not really understanding as you point out shouldn't be instantitiating the default constructor. In any case, that's an MSVC and/or standard library issue.

I do see the code in collections_load_impl.hpp you refer to.

So ... what do you want me to do? Do you have a patch to submit?

Robert Ramey

comment:7 by Robert Ramey, 12 years ago

Resolution: wontfix
Status: reopenedclosed

I"m going to mark this as "can't fix" since I can't do anything with it.

Robert Ramey

in reply to:  5 ; comment:8 by boost@…, 11 years ago

Replying to ramey:

I do indeed get a compiler error on the following line

std::map<int,A>::value_type m1(1,A(10));

which I'm not really understanding as you point out shouldn't be instantitiating the default constructor. In any case, that's an MSVC and/or standard library issue.

Robert, we've also seen this. It is indeed MSVC getting confused, resulting in it thinking a default constructed pair must be required.

However, it IS triggered by the attempt to serialize a std::pair where at least one of the arguments has a private default ctor. This can be seen by commenting out lines 60 and 65 (as they appear in trac) in the OP's test.cpp.

So ... what do you want me to do? Do you have a patch to submit?

Because the key or value type of the std::pair will (typically) have been given friend access to boost::serialization::access, it is annoying that the pair trips up. However, the following appears to be a suitable patch (unless I've missed something):

Add to boost/serialization/utility.hpp:

template <typename Archive, typename First, typename Second> void load_construct_data(Archive & ar, std::pair<First, Second> * t, unsigned int const) {

typedef BOOST_DEDUCED_TYPENAME remove_const<First>::type NonConstFirst; NonConstFirst &first = const_cast<NonConstFirst&>(t->first); access::construct(&first); access::construct(&t->second);

}

/

I.e. instead of invoking the default ctor of std::pair<>, route through boost::serialization::access to in-place construct the two values.

in reply to:  8 comment:9 by boost@…, 11 years ago

Replying to boost@…:

Forgot formatting:

template <typename Archive, typename First, typename Second>
void load_construct_data(Archive & ar, std::pair<First, Second> * t, unsigned int const)
{
    typedef BOOST_DEDUCED_TYPENAME remove_const<First>::type NonConstFirst;
    NonConstFirst &first = const_cast<NonConstFirst&>(t->first);
    access::construct(&first);
    access::construct(&t->second);
}

comment:10 by anonymous, 11 years ago

Same problem here.

The culprit is with MSVC reporting an error at the template instantiation point, instead of pointing to the actual place where the default constructor of std::pair (and thus of the UDT without default constructor) is used.

Note: See TracTickets for help on using tickets.