Re: [boost] Formal Review Request: TypeErasure

Subject: Re: [boost] Formal Review Request: TypeErasure
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2012-05-23 11:39:01

AMDG

On 05/23/2012 07:31 AM, Larry Evans wrote:
> On 05/22/12 15:34, Steven Watanabe wrote:
> [snip]
>> Online documentation can be found here:
>> http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/
>>
> [snip]
> Looks very interesting.
> 
> I've done a somewhat brief review of just the online documentation.
> I'll try to do a more complete review later.
> 
> Here's my current comments:
> 
> 
> * Virtual functions do not *require* dynamic memory:
> 
>   The page:
> 
> 
> http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/doc/html/index.html
> 
> 
>   says:
> 
>     C++ provides runtime polymorphism through virtual functions. ...
>     They require dynamic memory management.
> 
>   However, I don't think dynamic memory is required because just
>   references to stack locations or pointers pointing to stack
>   locations could be used to take advantage of virtual functions.
> 

In theory, you're correct.  It isn't strictly
necessary.  However, in practice, the cases
where you can avoid new are the exception,
rather than the rule.  I don't want to complicate
the introduction by being overly pedantic.

> * 'Operation' term used before defined:
> 
>   The page:
> 
> 
> http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/doc/html/boost_typeerasure/multi.html
> 
> 
>   says:
> 
>     Operations can have more than one any argument.
> 
>   before any definition of 'Operation' occurs.  Only by seeing the:
> 

"Operation" is not defined, because it doesn't
have any special formal meaning in my library.
Just take it in it's usual sense of something
that you can do with an any.

>     addable<>
> 
>   argument to the mpl::vector does the term 'Operation' begin to make
>   sense, and then the reader has to guess that all the mpl::vector
>   arguments are operations, and that all the previous 'operations'
>   were unary.
> 

The very next sentence explicitly contrasts *binary*
addition with increment.  I don't see how I can make
this more clear without a lot of formalism that
would just be off-putting for most people.

>   However, I'm still not sure that's correct. It would help if some
>   link to 'Operation' defintion were provided everywhere that the
>   term, 'Operation', occurs.
> 
>   Maybe, where the term, 'opeation" occurs, there should be a link to:
> 
> 
> http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/doc/html/boost_typeerasure/predef.html
> 
> 
> * 'match' term used before defined:
> 
>   The page:
> 
> 
> http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/doc/html/boost_typeerasure/multi.html
> 
> 
>   says:
> 
>     The types of the arguments must match or the behavior is
>     undefined.
> 
>   before any definition of 'match' or 'arguments' occurs.  I'm just
>   guessing from the arguments to x and y, that 'arguments' means
>   arguments to the any CTOR, but then the CTOR argument to z is an
>   expression; hence, I'm guessing that that expression is allowed by
>   the addable<> template argument to the mpl::vector arg to the
>   any_type.
> 

arguments in this case means the arguments of +.
types means the types that the anys hold.
match just means that they're consistent
with what addable<> expects i.e. the same type.
This is the *tutorial*.  I don't want to
put all the technical details here.  If you
come away from this sentence with the idea
that adding an any that stores an int to
an any that stores a string is bad, it's
clear enough for my purposes.

> * public construct/copy/destruct item 1 undefined T:
> 
>   The page:
> 
> 
> http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/doc/html/boost/type_erasure/any.html
> 
> 
>   under:
> 
>     1. template<typename U> explicit any(const U & data);
> 
>   says:
> 
>     Concept must not refer to any placeholder besides T.
> 
>   However, T is not defined (or the definition is not obvious).
> 

T is a class template parameter.  I'm
not sure what I can do to make it more
obvious.  Anyway, this comment is no
longer correct.

> * Mistitled Assign operator description:
> * Misplaced copy CTOR description:
> 

This is all auto-generated by doxygen/boostbook.
I'll see if I can figure out what went wrong.

> 
> * Typo "more general" in 'Design Notes.Constructors':
> 
>   The page:
> 
> 
> http://steven_watanabe.users.sourceforge.net/type_erasure/libs/type_erasure/doc/html/boost_typeerasure/design.html
> 
> 
>   says:
> 
>     The last kind of constructor "upcasts" from one concept to another
>     more general more general concept.
> 
>   with the typo of duplicated "more general".
> 

Fixed.

In Christ,
Steven Watanabe