#11245 closed Bugs (fixed)
Deserialization with private constructor works in 1.57 not in 1.58
Reported by: | Thomas Aho | Owned by: | Robert Ramey |
---|---|---|---|
Milestone: | To Be Determined | Component: | serialization |
Version: | Boost 1.58.0 | Severity: | Problem |
Keywords: | Cc: | stefan.hoelldampf@…, visa.jokelainen@… |
Description
Under certain circumstances deserialization of a class with a private constructor does not work with Boost 1.58. In 1.57 it worked fine. The code in the attached file, when compiled with Visual Studio 2013, gives the following error message:
c:\program files (x86)\vc\include\xmemory0(588): error C2248: 'A::A' : cannot access private member declared in class 'A'
1> c:\projects\serializetest\serializetest.cpp(14) : see declaration of 'A::A'
1> c:\projects\serializetest\serializetest.cpp(9) : see declaration of 'A'
Attachments (2)
Change History (10)
by , 7 years ago
comment:1 by , 7 years ago
Cc: | added |
---|
comment:2 by , 7 years ago
Turns out that I've had to look into the serialization of std::vector due to an unrelated issue. Here is what I found:
serialization of a type with a private constructor will fail if that constructor needs to be called.
But that often doesn't happen:
a) on most common serialization the type is already created and just "filled in" by the load.
b) when serialized through a pointer, the same type will fail as this requires that a new instance be created.
c) An interesting case is std::vector the manner of loading is dependent on the result of the is_default_constructible type_trait. So I'd have to look at that again to see the effect of a constructor being private.
This is even worse as the implementation of "is_default_constructible" is buggy in some compilers, and in others its correct but marked as buggy by boost/config.hpp.
Now that I know all this, I'm inclined to take another look. On one hand, I'm tempted to also check the size of the vector, if it's already big enough then I could just fill it in if it's not could fill in what I can and append to the end. On the other hand, that would introduce non-obvious quirky runtime behavior which I generally like to avoid. The current system always works or always fails at compile time.
Feel free to think about this some more and get back to me. I'll leave the ticket open for now.
by , 7 years ago
Attachment: | not_default_constructible.cpp added |
---|
Shows v1.58.0 compilation failure on attempt to serialize vector of non-default-constructible class
comment:3 by , 7 years ago
Extra information...
This problem doesn't just affect classes with private constructors; I'm finding it also breaks compilation for serialization of vectors of classes without default constructors (but with suitable load_construct_data()
specializations).
I've attached code (attachment:not_default_constructible.cpp) that compiles cleanly under v1.57.0:
g++ -std=c++1y -c -o ndc.o -isystem /opt/boost_1_57_0_gcc_build/include -ftemplate-backtrace-limit=0 not_default_constructible.cpp clang++ -std=c++1y -stdlib=libc++ -c -o ndc.o -isystem /opt/boost_1_57_0_clang_build/include -ftemplate-backtrace-limit=0 not_default_constructible.cpp
...but that fails to compile under v1.58.0:
g++ -std=c++1y -c -o ndc.o -isystem /opt/boost_1_58_0_gcc_build/include -ftemplate-backtrace-limit=0 not_default_constructible.cpp clang++ -std=c++1y -stdlib=libc++ -c -o ndc.o -isystem /opt/boost_1_58_0_clang_build/include -ftemplate-backtrace-limit=0 not_default_constructible.cpp
Both GCC and Clang fail whilst compiling the call to the vector's resize()
at line 90 of boost/serialization/vector.hpp
.
My hypothesis is as follows... The changes to load()
in boost/serialization/vector.hpp
since v1.57.0 introduce problems because they add code that requires the type to be default constructible. Though it's surrounded by an if(detail::is_default_constructible<U>()){ [...] }
, I think that's insufficient to stop it getting compiled. If it's important to keep this separate code for default constructible types, perhaps some sort of traits/SFINAE/template-specialization approach is required?
Many thanks to all who contribute effort to this.
comment:4 by , 7 years ago
Cc: | added |
---|
comment:5 by , 7 years ago
Note that in the version in the develop branch, I altered the implementation so that it only generates code actually used. This fixes the problem on my Clang, GCC compilers. So I believe the problem is "addressed". However, it does leave the confusion that occurs with a private ctor. That is, the error message refers to a private ctor which is a little misleading.
Note that the issue of loading type T with a private default cort or not default constructible are related. That is the former implies the latter. This seems obvious in retrospect but certainly got me confused.
I'm also thinking that loading a vector<T> where T is not default constructible should probably be called as a compile time error even though it could pass at runtime using the "fill-in" idea above.
Soooooo, all in all, I think that the deserialization a vector whose type is not default constructible should be considered an error even though previously it was OK.
comment:6 by , 7 years ago
I'm not sure I fully understand the argument in comment:5 but, FWIW, I'd vote that it's worth continuing to support serialization/deserialization of vector<T> for non-default-constructible T (as was previously supported in v1.57.0).
Reasons:
- AFAIK, the standard doesn't require T to be default-constructible,
- the serialization library allows users to define how the library should serialize/deserialize their non-default-constructible types and
- the library previously supported this in v1.57.0.
comment:7 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
The same error here on Fedora 21 with gcc-4.9.2.
I think the problem is related to the fact that load() in boost/serialization/vector.hpp calls detail::is_default_constructible<U>(), but it does not check whether the default constructor is accessible.
Eventually, std::vector::resize() fails.