#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)
Change History (12)
by , 12 years ago
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:3 by , 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 , 12 years ago
Attachment: | test_g++.out added |
---|
comment:4 by , 12 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
follow-ups: 6 8 comment:5 by , 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
comment:6 by , 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 , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
I"m going to mark this as "can't fix" since I can't do anything with it.
Robert Ramey
follow-up: 9 comment:8 by , 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.
comment:9 by , 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 , 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.
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