Re: [boost] [serialization] load_construct proposal

From: Dave Harris (brangdon_at_[hidden])
Date: 2003-10-12 20:58:00

In-Reply-To: <01C390D0.1B1B09E0_at_[hidden]>
ramey_at_[hidden] (Robert Ramey) wrote (abridged):
> Below I will try to summarize the argument on this thread.

Clearly you don't like this idea. I think further advocacy from me would 
not be productive. However, I can't resist commenting on your summary.

> Here is Dave Harris's proposal:
> /////////////////////
> 
> template<class Archive ar, class T>
> void load_construct(Archive & ar,  T * t,  unsigned int file_version)
> {
>     new( pt ) T(); // Default-construct in place
>     ar >> *t;    // presumably
> }

This has evolved into something more like:

 template<class Archive ar, class T>
 T *load_construct(Archive & ar,  void * pt,  unsigned int file_version)
 {
     T *t = new( pt ) T();
     try {
         ar >> *t;
     } catch (...) {
         t->T::~T();
         throw;
     }
     return t;
 }

The signature has changed as well as the implementation. There is some 
question about the use of placement new here, in the cases where the user 
has class-specific operator new. This is a general issue. In my view 
boost, and/or the C++ standard, needs to evolve support for safe use of 
placement new.

> template<class Archive ar, class T>
> void load(Archive & ar,  T * &t,  unsigned int file_version)
> {
>     t = NULL; 
>     try{  
>        p = static_cast<T *>(new char[sizeof(T)]); // allignment issues?
>        load_construct(ar, static_cast<T *>vp, file_version)
>     }
>     catch(...){
>         delete t; // hmm - what about dtor? 
>         throw;
>     }
> }

This becomes:

 template<class Archive ar, class T>
 void load(Archive & ar,  T * &t,  unsigned int file_version)
 {
     char *pt = NULL;
     try{  
        pt = new char[sizeof(T)];
        t = load_construct<Archive,T>(ar, pt, file_version)
     }
     catch(...){
         delete pt;
         throw;
     }
 }

There are no alignment issues. Operator new is required to return memory 
with suitable alignment. There are no casts. The T destructor is already 
taken care of.

> 4) replace overriable functions save/load with  
> save_construct/load_construct.
> These don't convey meanings of what they do. ( what could save_construct
> possibly mean?)

Use different names if you prefer. Eg save_uninitialised/ 
load_uninitialised. I have argued that the current reuse of "load" is 
misleading because the function is doing more than loading a pointer. It 
is also allocating memory, constructing an object, and loading it. I think 
it is important that functions which do very different things should have 
different names.

Your list omits the main motivation and benefit, ie support for 
non-intrusive serialisation of standard containers. I would argue that the 
serialisation fails to fulfill its design objectives without it. The 
alternatives involve either providing a default constructor, or a friend 
declaration, or rewriting the container serialisation functions for each 
T.

The first two require changes to T - with them the library is intrusive. 
Where that isn't possible I think you have in mind rewriting the container 
serialisation. Which is OK, but it makes this:

> 3) without making the public api larger, does not permit
>       a) override of memory allocation
>       b) overide of exception handlling

seem unjust. With your approach, container serialisation must be part of 
the API.

Note that 3.a and 3.b are not on the list of design objectives, but 
non-intrusive serialisation of standard containers without undue source 
code bloat is. I don't really get 3.b; both proposals pass all exceptions 
back to the caller to be handled. 

-- Dave Harris, Nottingham, UK