Re: [boost] [1.44] Beta progress?

Subject: Re: [boost] [1.44] Beta progress?
From: Matthias Troyer (troyer_at_[hidden])
Date: 2010-07-26 12:09:28

On 25 Jul 2010, at 23:56, Robert Ramey wrote:

> Matthias Troyer wrote:
>> 
>> I have one more question in addition to my previous comment:
>> 
>> common_oarchive is in namespace archive::detail while
>> basic_binary_oarchive is in the top namespace archive.
> 
>> Do I  understand you correctly that deriving from
>> archive::detail::common_oarchive is safe and not considered depending
>> on implementation details, while deriving from
>> archive::basic_binary_oarchive is not?
>> 
>> I can easily change all the Boost.MPI archives to use
>> archive::detail::common_oarchive where they now use
>> archive::basic_binary_oarchive (although this will not solve the
>> issue we have right now).
> 
> I can see where this would be confusing.  Let me indicate what I mean
> of a few of the terms being used.
> [...]
> 
> 
> And the stuff in "detail" has it's own "public" and
> "implemention detail" aspects.  For an archive developer
> who wants to develope an archive with functionality
> similar to the existing ones, it's not a detail. He wants to know
> that the public functions aren't going to change.

Indeed - and what I need to know is which of these classes I may derive from safely. 
> 
> So - back to our problem.
> 
> I had thought the the source of the issue was coupling
> mpi_archive/skeleton to binary_archive implementation.  That's
> why I thought deriving from common_archive would
> help.
> 
> If I'm wrong about the above then deriving
> mpi_archive from common_archive won't help - though
> it would probably be a good idea.

Indeed, it does not help but I still changed it to (hopefully) avoid future issues. It would be good if you could add somewhere to the documentation which of the classes one can safely derive from.

> 
> If the only problem is that version_type eliminated
> some operations mpi_archive depended on all
> integer types to have (STRONG_TYPEDEF)
> this can also be worked out one way or the other
> without too much difficulty.
> 
> If all the above is true - this shouldn't be so hard
> to address.  Given that we've had so much
> difficulty with this, it's possible that one of the
> above is not true.
> 
> Finally, you've indicated that an archive writer needs
> to know the list of internal types in the archive
> and that they'll never change.  This would suggest
> to me that perhaps a separate section in the
> documentation describing the "common
> archive implementation" (text_archive, etc)distinct from
> other "sample implementations" (trivial archive, simple_log_archive
> etc.)
> 
> a description of the functionality of these archives.
> 
> Basically this supplies the missing "semantics" left
> undefined by the archive concept.  Basically
> it would list this functionaly, pointers, tracking
> versioning, etc.
> 
> common - this implemented special types
> used internally and their interface.  We can
> discuss whether these types should have the
> rich interface permited by STRONG_TYPEDEF
> or a narrow one which is useful for catching
> coding errors.
> 

Yes, what is missing is the list of special types and their interface. In the past they were all STRONG_TYPEDEF, and the semantics of STRONG_TYPEDEF was the same as that of the underlying integral type. That has changed and broken some code. 

The basic issue is that an archive deriving from detail::common_[io]archive has to support the fundamental C++ types, std:: string, and that collection of special types. In order to serialize those special types I need to be able to know something about them! The specific list of concepts they model is less important than that list being stable (as long as serialization can be implemented efficiently). Besides the default constructor it would be good to have a type member specifying the (current) implementation type:

#define BOOST_ARCHIVE_STRONG_TYPEDEF(T, D)         \
  class D : public T {                           \
  public:                                        \
      typedef T implementation_type;             \
      explicit D(const T t) : T(t){}             \
      D() : T() {}                               \
  };                                             \

and also use the implementation_type member in the other special classes. That way you could change those integer types at any time without causing any breakage in Boost.MPI. Right now I have to manually set these types:

BOOST_MPI_DATATYPE(boost::archive::class_id_optional_type, get_mpi_datatype(int_least16_t()), integer);

and if you should change class_id_optional_type to be something else than int_least16_t the code will break again.

Matthias