Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#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)

boost.cpp (1.1 KB ) - added by anonymous 7 years ago.
not_default_constructible.cpp (996 bytes ) - added by Tony Lewis <tonyelewis@…> 7 years ago.
Shows v1.58.0 compilation failure on attempt to serialize vector of non-default-constructible class

Download all attachments as: .zip

Change History (10)

by anonymous, 7 years ago

Attachment: boost.cpp added

comment:1 by Stefan Hoelldampf <stefan.hoelldampf@…>, 7 years ago

Cc: stefan.hoelldampf@… added

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.

comment:2 by Robert Ramey, 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 Tony Lewis <tonyelewis@…>, 7 years ago

Shows v1.58.0 compilation failure on attempt to serialize vector of non-default-constructible class

comment:3 by Tony Lewis <tonyelewis@…>, 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 visa.jokelainen@…, 7 years ago

Cc: visa.jokelainen@… added

comment:5 by anonymous, 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 Tony Lewis <tonyelewis@…>, 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 Robert Ramey, 7 years ago

Resolution: fixed
Status: newclosed

comment:8 by visa.jokelainen@…, 7 years ago

Hi.

Any idea whether this will be released with boost 1.59.0?

-Visa

Note: See TracTickets for help on using tickets.