Re: [boost] [type_erasure] Review

Subject: Re: [boost] [type_erasure] Review
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2012-07-26 17:22:51

AMDG

On 07/26/2012 12:20 PM, Karsten Ahnert wrote:
> Hi,
> 
> here is my review for Steven Watanabe's TypeErase library.
> 
> First of all, I want to thank Steven for developing this library. My
> overall evaluation is that TypeErasue should be accepted as boost
> library. I did not looked very deeply into all details or the
> implementation but I tried to solve some problems with the library. So,
> I only looked at the library from the users perspective.
> 
> 
> 
> = 1. What is your evaluation of the design? =
> 
> The design looks clear and concise. I have only two small point:
> 
> * Why do I need to specify relaxed_match to get a default constructable
> any?

Except for the absolutely essential components,
(e.g. binding_of, concept_of, placeholder_of,
any -> any conversions), all behavior of an any
must be specified explicitly. I bundled a lot
of functionality into relaxed match.  (the default
constructor, assignment in terms of copy_constructible).
I could split it up, if it's needed, but
I assumed that all these things that make
any act more like a normal object are
likely to be used together.

> Maybe a empty() function would be nice to check if any holds a
> value (in case it is default constructable).

Okay.

> I think a hint about a
> default constructable any should be in the tutorial. It is important for
> users.
> 
> * Sometimes it is not clear, why some constructors work and others not,
> at least for me. Details are shown below.
> 
> 
> 
> = 2. What is your evaluation of the implementation? =
> 
> I did not looked into the implementation.
> 
> 
> 
> = 3. What is your evaluation of the documentation? =
> 
> * The documentation looks good. But I think more real-world examples
> would be very useful. I also encountered some problems where I could not
> find a hint how to solve it. For example, consider the case where you
> need an any (for example in a recursive data structure) to be a member
> of some other class:
> 
> struct my_class
> {
> 
> private:
>    any_type a;
> };
> 
> What kind of constructor do I need to set up this any properly? I came
> up to define two constructors:
> 
> struct my_class
> {
>     template< class T >
>     my_class( T t ) : a( std::move( t ) ) { }
>     my_class( any_type a_ ) : a( a_ ) { }
> private:
>     any_type a;
> };
> 

I'm not sure why you need this.  Either
constructor should be sufficient.

> 
> template< class Context > class not_ : struct unary_expr< bool , Context > {
>     template< class T > not_( T child ) : unary_expr< bool , Context >(
> child ) { }
>     bool operator()( Context c ) { return ! this->m_child( c ); }
> };
> 

I think this is ill-formed with Concept = void.

> 
> This works quite nice, but I always had problems to figure out how the
> any type are constructed, for example
> 
> using expr_type = boolean_expr< void >;
> using not_ = not_< void >;
> using and_ = and_< void >;
> using or_ = or_< void >;
> 
> true_ t;
> false_ f;
> 
> // why does this not work?
> // expr_type e3( not_( f ) );
> // cout << e3() << endl;
> 
> // why does this work?
> expr_type e4( and_( t , or_( f , t ) ) );
> cout << e4() << endl;
> 
> The first example expr_type e3( not_( f ) ) does not compile. It fails
> because not_( f ) is interpreted as a function pointer:
> 

e3 is a function declaration.  It's equivalent to:
expr_type e3(not_ f);

> rule_engine.cpp:35:16: error: too few arguments to function ‘expr_type
> e3(not_)’
> rule_engine.cpp:34:15: note: declared here
> 
> I have no idea why the second one compiles.
> 

e4 can't be parsed as a function declaration.

In Christ,
Steven Watanabe