[boost] Re: Serialization Formal Review #2

From: Fredrik Blomqvist (fredrik_blomqvist_at_[hidden])
Date: 2004-04-25 19:12:18

Hi

This is my re-review of the Serialization library. My previous review can be
found here: http://lists.boost.org/MailArchives/boost/msg39608.php

Most of the issues I previously had have been adressed in this version.

----------------------------------------------------------------------------
-------

* What is your evaluation of the design?
Very good. It gives great flexibility to the user while still being very
efficient. Compared to the previous version up for review this has become
even better. At that time I wasn't so concerned by archive formats but has
since come to appreciate the large improvements in that area. The new synax
& interface for serialization is also very nice.

* What is your evaluation of the implementation?
As I mentioned in my previous review I would like to see it handle most of
the STL and core boost components "out of the box". (see my attachment also)
In particular I think the smart_ptrs (including auto_ptr) should have
core-support from the serializer, even special cased if necessary (similar
to boost::bind etc). The code to serialize std::auto_ptr could easily be
copied to the official directory for example.

Regarding shared_ & weak_ptr. If the implementation path Robert outlines in
the documentation is to be used I guess an official patch to shared_count is
necessary. Would be interesting to hear what the smart pointer experts
think.
Otherwise, I still think the solution I outlined in my previous review that
doesn't require modifying shared_ptr could be considered. Downside beeing
that it most likely adds a special-cased codepath to the serializer, but as
I said I think it would be worth it.

The code is nicely written altough complex. Highly nested template code and
lots of mpl. All compile-time checks and further factorizations are
valuable.

Nitpick: serialization/scoped_ptr.hpp seems to have a typo. An extra #pragma
once is present outside the #ifdef (_MSC..) block. It also includes
serialization/pfto.hpp but doesn't seem to use it.

* Parts of the library that might become standalone boost libraries
The library already provides good separation of components. static_warning,
strong typedef, pfto, stack_allocate etc are all given candidates that would
just need better documentation and (fast-track?) reviews.

* What is your evaluation of the documentation?
I would also like to see an improved reference documentation that clearly
states the requirements on the types/concepts involved.
I support the separation of documentation (and implementation) of other
boost components that like to support serialization. A simple index with
links would suffice I think but most importantly a _consistent_ structure
should be agreed upon which new components can follow.

Nitpick: bibliography link to CommonC++ serializer is broken; should be:
http://www.gnu.org/software/commoncpp/

* What is your evaluation of the potential usefulness of the library?
Very useful! I'm looking forward to using the library in many projects to
come.

* Did you try to use the library?  With what compiler?
Yes. I used VC7.1. I compiled the review version with a current boost CVS
and the only thing I had to change was to #ifdef the duplicate
boost::is_abstract code.
I would appreciate VC7.1 project files for the examples and tests though.
One of my tests added support for the STL container adaptors std::stack,
queue and priority_queue. I think direct support for them could be
convinient since they hide the native containers. I attach my
implementation. Feel free to comment. (The implementation could be
considered to employ premature optimization.. ;-) )

* How much effort did you put into your evaluation?
* Are you knowledgeable about the problem domain?
Moderate. I've used the MFC serializer, developed and used an in-house "MFC
copy", and used the CommonC++ Persist lib to which I added STL and
boost::smart_ptr support.
I've been trying out several of the previous versions of Robert's library
ever since he first announced it on the boost-list almost two years ago.

* Do you think the library should be accepted as a Boost library?
Yes!

Thanks for a fine library!
// Fredrik Blomqvist

begin 666 queue.hpp
M(VEF;F1E9B!"3T]35%]315))04Q)6D%424].7U%5155%7TA04 T*(V1E9FEN
M92!"3T]35%]315))04Q)6D%424].7U%5155%7TA04 T*#0HC:68_at_9&5F:6YE
M9"A?35-#7U9%4BD@)B8@*%]-4T-?5D52(#X](#$P,C I#0HC('!R86=M82!O
M;F-E#0HC96YD:68-"@T*+R\@0V]P>7)I9VAT("A#*2 R,# T($9R961R:6L@
M0FQO;7%V:7-T+@T*+R\@57-E+"!M;V1I9FEC871I;VX_at_86YD(&1I<W1R:6)U
M=&EO;B!I<R!S=6)J96-T('1O('1H92!";V]S="!3;V9T=V%R90T*+R\@3&EC
M96YS92P_at_5F5R<VEO;B Q+C N("A3964_at_86-C;VUP86YY:6YG(&9I;&4_at_3$E#
M14Y315\Q7S N='AT(&]R(&-O<'D_at_870-"B\O(&AT=' Z+R]W=W<N8F]O<W0N
M;W)G+TQ)0T5.4T5?,5\P+G1X="D-"@T*+R\@4')O=FED97,@;F]N+6EN=')U
M<VEV92!S97)I86QI>F%T:6]N(&9O<B!S=&0Z.G%U975E(&%N9"!S=&0Z.G!R
M:6]R:71Y7W%U975E( T*+R\-"B\O(%1H97-E(&%R92!C;VYT86EN97(@7V%D
M87!T;W)S7R!A;F0_at_049!24L@=&AE<F4G<R!N;R!C;VYF;W)M:6YG('=A>2!T
M;R!A8V-E<W,-"B\O('1H92!U;F1E<FQY:6YG(&-O;G1A:6YE<B!W:71H;W5T
M('1H:7,@97AT<F$@8V]P>6EN9R!S=&5P+BX@#0HO+R!!=&QE87-T($1I;FMU
M;7=A<F4_at_4U1,('=I=&@@5D,W+C$@<V5E;7,@=&\@;&5A=F4@<W1D.CIQ=65U
M93HZ8R!P=6)L:6,@=&AO=6=H#0HO+R!W:&EC:"!M:6=H="!B92!W;W)T:"!A
M9&1I;F<@(VEF9&5F<R!F;W(N#0H-"@T*(VEN8VQU9&4@/'%U975E/@T*#0HC
M:6YC;'5D92 \8F]O<W0O<V5R:6%L:7IA=&EO;B]S<&QI=%]F<F5E+FAP<#X-
M"@T*#0IN86UE<W!A8V4_at_8F]O<W0@>R!N86UE<W!A8V4@<V5R:6%L:7IA=&EO
M;B![#0H-"@T*;F%M97-P86-E(&1E=&%I;"![#0H-"B @("!T96UP;&%T92 \
M#0H@(" @(" @(&-L87-S(%0L(&-L87-S($-O;G0L(&-L87-S($-O;7 -"B @
M(" ^#0H@(" @8VQA<W,@<')I;U]H86-K(#H@<'5B;&EC('-T9#HZ<')I;W)I
M='E?<75E=64\5"P_at_0V]N="P_at_0V]M<#X-"B @("![#0H@(" @(" @('!R:6]?
M:&%C:RAP<FEO7VAA8VL\5"Q#;VYT+$-O;7 ^(&-O;G-T("8I.PT*(" @(" @
M("!P<FEO7VAA8VL\5"Q#;VYT+$-O;7 ^)B!O<&5R871O<CT@*'!R:6]?:&%C
M:SQ4+$-O;G0L0V]M<#X_at_8V]N<W0@)BD[#0H-"B @("!P=6)L:6,Z#0H@(" @
M(" @('1Y<&5D968@<W1D.CIP<FEO<FET>5]Q=65U93Q4+"!#;VYT+"!#;VUP
M/@T*(" @(" @(" @(" @<')I;U]T>7!E.PT*#0H@(" @(" @('1Y<&5D968@
M='EP96YA;64@<')I;U]T>7!E.CIC;VYT86EN97)?='EP90T*(" @(" @(" @
M(" @8V]N=&%I;F5R7W1Y<&4[#0H-"B @(" @(" @<')I;U]H86-K*"D@>WT-
M"@T*(" @(" @("!E>'!L:6-I="!P<FEO7VAA8VLH<')I;U]T>7!E(&-O;G-T
M)B!P<2D@#0H@(" @(" @(" @(" Z('!R:6]?='EP92AP<2D-"B @(" @(" @
M>WT-"@T*(" @(" @("!C;VYT86EN97)?='EP928_at_9V5T7V-O;G1A:6YE<B_at_I
M#0H@(" @(" @('L-"B @(" @(" @(" @(')E='5R;B!C.PT*(" @(" @("!]
M#0H@(" @?3L-"@T*#0H@(" @=&5M<&QA=&4@/ T*(" @(" @("!C;&%S<R!4
M+"!C;&%S<R!#;VYT#0H@(" @/@T*(" @(&-L87-S('%U975E7VAA8VL_at_.B!P
M=6)L:6,@<W1D.CIQ=65U93Q4+"!#;VYT/@T*(" @('L-"B @(" @(" @<75E
M=65?:&%C:RAQ=65U95]H86-K/%0L0V]N=#X_at_8V]N<W0@)BD[#0H@(" @(" @
M('%U975E7VAA8VL\5"Q#;VYT/B8@;W!E<F%T;W(]("AQ=65U95]H86-K/%0L
M0V]N=#X_at_8V]N<W0@)BD[#0H-"B @("!P=6)L:6,Z#0H@(" @(" @('1Y<&5D
M968@<W1D.CIQ=65U93Q4+"!#;VYT/@T*(" @(" @(" @(" @<75E=65?='EP
M93L-"@T*(" @(" @("!T>7!E9&5F('1Y<&5N86UE('%U975E7W1Y<&4Z.F-O
M;G1A:6YE<E]T>7!E#0H@(" @(" @(" @("!C;VYT86EN97)?='EP93L-"@T*
M(" @(" @("!Q=65U95]H86-K*"D@>WT-"@T*(" @(" @("!E>'!L:6-I="!Q
M=65U95]H86-K*'%U975E7W1Y<&4_at_8V]N<W0F('$I( T*(" @(" @(" @(" @
M.B!Q=65U95]T>7!E*'$I( T*(" @(" @("![?0T*#0H@(" @(" @(&-O;G1A
M:6YE<E]T>7!E)B!G971?8V]N=&%I;F5R*"D-"B @(" @(" @>PT*(" @(" @
M(" @(" @<F5T=7)N(&,[#0H@(" @(" @('T-"B @("!].PT*#0H-"GT@+R\@
M;F%M97-P86-E(&1E=&%I; T*#0H-"G1E;7!L871E/ T*(" @(&-L87-S($%R
M8VAI=F4L( T*(" @(&-L87-S(%0L(&-L87-S($-O;G0-"CX@#0IV;VED('-A
M=F4H#0H@(" @07)C:&EV928_at_87(L( T*(" @('-T9#HZ<75E=64\5"Q#;VYT
M/B!C;VYS="8@<2P@#0H@(" @=6YS:6=N960@:6YT(&-O;G-T('9E<G-I;VX-
M"BE[#0H@(" @87(@/#P_at_9&5T86EL.CIQ=65U95]H86-K/%0L0V]N=#XH<2DN
M9V5T7V-O;G1A:6YE<B_at_[hidden]*?0T*#0IT96UP;&%T93P-"B @("!C;&%S<R!!
M<F-H:79E+" -"B @("!C;&%S<R!4+"!C;&%S<R!#;VYT#0H^#0IV;VED(&QO
M860H#0H@(" @07)C:&EV928_at_87(L( T*(" @('-T9#HZ<75E=64\5"Q#;VYT
M/B8@<2P@#0H@(" @=6YS:6=N960@:6YT(&-O;G-T('9E<G-I;VX-"BE[#0H@
M(" @9&5T86EL.CIQ=65U95]H86-K/%0L0V]N=#X_at_8SL-"B @("!A<B ^/B!C
M+F=E=%]C;VYT86EN97(H*3L@#0H@(" @<2 ](&,[#0I]#0H-"G1E;7!L871E
M/ T*(" @(&-L87-S($%R8VAI=F4L( T*(" @(&-L87-S(%0L(&-L87-S($-O
M;G0-"CX-"G9O:60@<V5R:6%L:7IE* T*(" @($%R8VAI=F4F(&%R+" -"B @
M("!S=&0Z.G%U975E/%0L($-O;G0^)B!Q+" -"B @("!U;G-I9VYE9"!I;G0@
M8V]N<W0@=F5R<VEO;@T**7L-"B-I9B Q#0H@(" @<W!L:71?9G)E92AA<BP@
M<2P@=F5R<VEO;BD[#0HC96QS90T*"2\O(&ES('1H:7,@86QL;W=E9"]S869E
M/PT*"6%R("8@<W1A=&EC7V-A<W0\9&5T86EL.CIQ=65U95]H86-K/%0L($-O
M;G0^)CXH<2DN9V5T7V-O;G1A:6YE<B_at_[hidden]*(V5N9&EF#0I]#0H-"B\O+R\O
M+R\O+R\O#0H-"G1E;7!L871E/ T*(" @(&-L87-S($%R8VAI=F4L( T*(" @
M(&-L87-S(%0L(&-L87-S($-O;G0L(&-L87-S($-O;7 -"CX-"G9O:60@<V%V
M92_at_-"B @("!!<F-H:79E)B!A<BP@#0H@(" @<W1D.CIP<FEO<FET>5]Q=65U
M93Q4+$-O;G0L0V]M<#X_at_8V]N<W0F('$L( T*(" @('5N<VEG;F5D(&EN="!C
M;VYS="!V97)S:6]N#0HI>PT*(" @(&%R(#P\(&1E=&%I;#HZ<')I;U]H86-K
M/%0L0V]N="Q#;VUP/BAQ*2YG971?8V]N=&%I;F5R*"D[#0I]#0H-"G1E;7!L
M871E/ T*(" @(&-L87-S($%R8VAI=F4L( T*(" @(&-L87-S(%0L(&-L87-S
M($-O;G0L(&-L87-S($-O;7 -"CX-"G9O:60@;&]A9"@-"B @("!!<F-H:79E
M)B!A<BP@#0H@(" @<W1D.CIP<FEO<FET>5]Q=65U93Q4+$-O;G0L0V]M<#XF
M('$L( T*(" @('5N<VEG;F5D(&EN="!C;VYS="!V97)S:6]N#0HI>PT*(" @
M(&1E=&%I;#HZ<')I;U]H86-K/%0L0V]N="Q#;VUP/B!C.PT*(" @(&%R(#X^
M(&,N9V5T7V-O;G1A:6YE<B_at_I.R -"B @("!Q(#T_at_8SL-"GT-"@T*=&5M<&QA
M=&4\#0H@(" @8VQA<W,@07)C:&EV92P@#0H@(" @8VQA<W,@5"P_at_8VQA<W,@
M0V]N="P_at_8VQA<W,@0V]M< T*/@T*=F]I9"!S97)I86QI>F4H#0H@(" @07)C
M:&EV928_at_87(L( T*(" @('-T9#HZ<')I;W)I='E?<75E=64\5"Q#;VYT+$-O
M;7 ^)B!Q+" -"B @("!U;G-I9VYE9"!I;G0_at_8V]N<W0@=F5R<VEO;@T**7L-
M"B @("!S<&QI=%]F<F5E*&%R+"!Q+"!V97)S:6]N*3L-"GT-"@T*?7T@+R\@
M;F%M97-P86-E(&)O;W-T+"!N86UE<W!A8V4@<V5R:6%L:7IA=&EO;@T*#0H-
J"B-E;F1I9B O+R!"3T]35%]315))04Q)6D%424].7U%5155%7TA04 T*
`
end

begin 666 stack.hpp
M(VEF;F1E9B!"3T]35%]315))04Q)6D%424].7U-404-+7TA04 T*(V1E9FEN
M92!"3T]35%]315))04Q)6D%424].7U-404-+7TA04 T*#0HC:68_at_9&5F:6YE
M9"A?35-#7U9%4BD@)B8@*%]-4T-?5D52(#X](#$P,C I#0HC('!R86=M82!O
M;F-E#0HC96YD:68-"@T*+R\@0V]P>7)I9VAT("A#*2 R,# T($9R961R:6L@
M0FQO;7%V:7-T+@T*+R\@57-E+"!M;V1I9FEC871I;VX_at_86YD(&1I<W1R:6)U
M=&EO;B!I<R!S=6)J96-T('1O('1H92!";V]S="!3;V9T=V%R90T*+R\@3&EC
M96YS92P_at_5F5R<VEO;B Q+C N("A3964_at_86-C;VUP86YY:6YG(&9I;&4_at_3$E#
M14Y315\Q7S N='AT(&]R(&-O<'D_at_870-"B\O(&AT=' Z+R]W=W<N8F]O<W0N
M;W)G+TQ)0T5.4T5?,5\P+G1X="D-"@T*+R\@4')O=FED97,@;F]N+6EN=')U
M<VEV92!S97)I86QI>F%T:6]N(&9O<B!S=&0Z.G-T86-K#0HO+PT*+R\@5&AI
M<R!I<R!A(&-O;G1A:6YE<B!?861A<'1O<E\@86YD($%&04E+('1H97)E)W,@
M;F\@8V]N9F]R;6EN9R!W87D@=&\@86-C97-S#0HO+R!T:&4@=6YD97)L>6EN
M9R!C;VYT86EN97(@=VET:&]U="!O;F4_at_97AT<F$@8V]P>6EN9R!S=&5P+BX@
M#0HO+R!!=&QE87-T($1I;FMU;7=A<F4_at_4U1,('=I=&@@5D,W+C$@<V5E;7,@
M=&\@;&5A=F4@<W1D.CIS=&%C:SHZ8R!P=6)L:6,-"B\O('=H:6-H(&UI9VAT
M(&)E('=O<G1H(&%D9&EN9R C:69D969S(&9O<BX@*$)/3U-47T1)3DM535=!
M4D5?4U1$3$E"/RD-"B\O($YO="!S=7)E(&ET)W,@=V]R=&@@=&AE(&UA:6YT
M96YA;F-E(&-O;G-T('1H;W5G:"XN#0H-"@T*(VEN8VQU9&4@/'-T86-K/@T*
M(VEN8VQU9&4@/&)O;W-T+W-E<FEA;&EZ871I;VXO<W!L:71?9G)E92YH<' ^
M#0H-"@T*;F%M97-P86-E(&)O;W-T('L@;F%M97-P86-E('-E<FEA;&EZ871I
M;VX@>PT*#0H-"FYA;65S<&%C92!D971A:6P@>PT*#0H@(" @=&5M<&QA=&4@
M/ T*(" @(" @("!C;&%S<R!4+"!C;&%S<R!#;VYT#0H@(" @/@T*(" @(&-L
M87-S('-T86-K7VAA8VL_at_.B!P=6)L:6,@<W1D.CIS=&%C:SQ4+"!#;VYT/@T*
M(" @('L-"B @(" @(" @<W1A8VM?:&%C:RAS=&%C:U]H86-K/%0L0V]N=#X@
M8V]N<W0@)BD[#0H@(" @(" @('-T86-K7VAA8VL\5"Q#;VYT/B8@;W!E<F%T
M;W(]("AS=&%C:U]H86-K/%0L0V]N=#X_at_8V]N<W0@)BD[#0H-"B @("!P=6)L
M:6,Z#0H@(" @(" @('1Y<&5D968@<W1D.CIS=&%C:SQ4+"!#;VYT/@T*(" @
M(" @(" @(" @<W1A8VM?='EP93L-"@T*(" @(" @("!T>7!E9&5F('1Y<&5N
M86UE('-T86-K7W1Y<&4Z.F-O;G1A:6YE<E]T>7!E#0H@(" @(" @(" @("!C
M;VYT86EN97)?='EP93L-"@T*(" @(" @("!S=&%C:U]H86-K*"D@>WT-"@T*
M(" @(" @("!E>'!L:6-I="!S=&%C:U]H86-K*'-T86-K7W1Y<&4_at_8V]N<W0F
M('!Q*2 -"B @(" @(" @(" @(#H@<W1A8VM?='EP92AP<2D@#0H@(" @(" @
M('M]#0H-"B @(" @(" @8V]N=&%I;F5R7W1Y<&4F(&=E=%]C;VYT86EN97(H
M*0T*(" @(" @("![#0H@(" @(" @(" @("!R971U<FX_at_8SL-"B @(" @(" @
M?0T*(" @('T[#0H-"GT@+R\@;F%M97-P86-E(&1E=&%I; T*#0H-"G1E;7!L
M871E/ T*(" @(&-L87-S($%R8VAI=F4L( T*(" @(&-L87-S(%0L(&-L87-S
M($-O;G0-"CX-"G9O:60@<V%V92_at_-"B @("!!<F-H:79E)B!A<BP@#0H@(" @
M<W1D.CIS=&%C:SQ4+$-O;G0^(&-O;G-T)B!Q+" -"B @("!U;G-I9VYE9"!I
M;G0_at_8V]N<W0@=F5R<VEO;@T**7L-"B @("!A<B \/"!D971A:6PZ.G-T86-K
M7VAA8VL\5"Q#;VYT/BAQ*2YG971?8V]N=&%I;F5R*"D[#0I]#0H-"G1E;7!L
M871E/ T*(" @(&-L87-S($%R8VAI=F4L( T*(" @(&-L87-S(%0L(&-L87-S
M($-O;G0-"CX-"G9O:60@;&]A9"@-"B @("!!<F-H:79E)B!A<BP@#0H@(" @
M<W1D.CIS=&%C:SQ4+$-O;G0^)B!Q+" -"B @("!U;G-I9VYE9"!I;G0_at_8V]N
M<W0@=F5R<VEO;@T**7L-"B @("!D971A:6PZ.G-T86-K7VAA8VL\5"Q#;VYT
M/B!C.PT*(" @(&%R(#X^(&,N9V5T7V-O;G1A:6YE<B_at_I.R -"B @("!Q(#T@
M8SL-"GT-"@T*=&5M<&QA=&4\#0H@(" @8VQA<W,@07)C:&EV92P@#0H@(" @
M8VQA<W,@5"P_at_8VQA<W,@0V]N= T*/@T*=F]I9"!S97)I86QI>F4H#0H@(" @
M07)C:&EV928_at_87(L( T*(" @('-T9#HZ<W1A8VL\5"Q#;VYT/B8@<2P@#0H@
M(" @=6YS:6=N960@:6YT(&-O;G-T('9E<G-I;VX-"BE[#0HC:68@,0T*(" @
M('-P;&ET7V9R964H87(L('$L('9E<G-I;VXI.PT*(V5L<V4-"@DO+PEA<B F
M('$N8SL-"@EA<B F('-T871I8U]C87-T/&1E=&%I;#HZ<W1A8VM?:&%C:SQ4
M+$-O;G0^)CXH<2DN9V5T7V-O;G1A:6YE<B_at_[hidden]*(V5N9&EF#0I]#0H-"@T*
M?7T@+R\@;F%M97-P86-E('-E<FEA;&EZ871I;VXL(&YA;65S<&%C92!B;V]S
M=" -"@T*#0HC96YD:68@+R\@0D]/4U1?4T5224%,25I!5$E/3E]35$%#2U](
$4% -"@``
`
end