Re: [Boost-bugs] [Boost C++ Libraries] #5341: Patch to improve shared library behavior with serialization

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #5341: Patch to improve shared library behavior with serialization
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2011-04-23 19:36:33

#5341: Patch to improve shared library behavior with serialization
----------------------------------------------+-----------------------------
  Reporter:  Aaron Barany <akb825@…>          |       Owner:  ramey                              
      Type:  Patches                          |      Status:  reopened                           
 Milestone:  To Be Determined                 |   Component:  serialization                      
   Version:  Boost 1.46.0                     |    Severity:  Problem                            
Resolution:                                   |    Keywords:  serialization shared_libraries dlls
----------------------------------------------+-----------------------------

Comment (by anonymous):

 I spent a little time looking at a couple of these patches in two files
 picked more or less at random;

 first basic_serializer_map.cpp

 a) it looks like you've excluded the possibility that the serialization
 for some class A may be instantiated in more than one DLL or in the
 mainline and DLL.  The comment you eliminated describes the "trap" which
 catches this ODR violation.  I had to disable the trap because too many
 users violate this rule and don't think it's a problem (i disagree) and
 they've found it too burdensome to re-factor their code to avoid
 violation.  In the future I may re-enable the trap with a method to
 suppress it for those who have problems with it.  So eliminating the
 extensive comment was a mistake.

 b) the adustment to use equal_range was clever. I have to admit I wasn't
 aware of this algorithm in the STL. In fact, I can't find it in the
 documention for G++.  This would make it a non-starter for portable code.

 c) you're function "equal_range" is just an implementation detail and
 would never need to be exported in any case.  A better strategy would be
 to use inline code as "type_compare" does in that same file.

 d) the change from returning a bool to void was of course correct and I'll
 consider including it

 second file:text_iarchive.hpp

 At first I thought this just moved the ...ARCHIVE macro from the header to
 to the *.cpp file.  which of course would be superflous.  Deeper
 investigation revealed that there was more involved with the definition of
 macros in other files and I lost track of it.  It did remind me that the
 explicit instantiation of the template archive_serializer_map<file> was a
 hack which I had to add to compensate for some other improvement/change. I
 realize that this is not really satisfactory - though I think my reason
 are different than yours - not to say yours are invalid.  I'm think about
 some required improvements to the library and addressing this would be a
 good idea.

 In general, I know you've invested a lot of effort in investigating and
 patching the library, but including these patches - even assuming they are
 all correct - would require days of work on my part which I can't spare
 right now.

 On the other hand, I don't want to dissuade people from helping me out on
 this and have incorporated many fixes and enhancements in the past (though
 not all of course).  If you're not too discouraged, you might consider
 separating your patches in smaller bites.  e.g.

 i) fix for archive_map instantiation.  Ideally it should not be necessary
 to include this in these archive.cpp files as it is an implementation
 detail.  My next enhancement will be to formalize the concepts of the
 archive classes so that it's easier to create correct versions of one's
 own archive classes and the current setup conflicts with that.  So I'm not
 totally adverse to this.

 i) improvements in efficiency of archive_map.  Of course, as noted above
 you're improvements wouldn't work for me.  But mixing multiple changes in
 the same set of patches makes the job of evaluating them very hard for me.

 ... etc.  I don't know what else is in here.

 also remember that the code is years old to me and to consider something
 like this I have to spend a lot of time re-acquainting myself with it. In
 a sense you'll have to gift wrap it for me.  I realize that this maybe not
 be fair (and probably isn't) but it's just a fact of life.

 I don't know if you're still interested in this, but if you are I'll share
 with you a list of the issues that I hope to see addressed in the near
 future with the eye that you might want to contribute.

 misc note: I use cygwin GCC to test my code with the gcc compiler etc.
 It's invaluable to get the code to pass on all compilers - which of course
 is a necessity for us.  It's also easy to download and install.

 I've spent a lot of time on this as I know you've spent a lot of time of
 your own and I want to respect that.  You've earned the right to a fair
 hearing.

 Robert Ramey

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/5341#comment:7>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.