Re: [boost] [type_erasure] Review

Subject: Re: [boost] [type_erasure] Review
From: Karsten Ahnert (karsten.ahnert_at_[hidden])
Date: 2012-07-27 02:32:13

On 07/26/2012 11:22 PM, Steven Watanabe wrote:
> 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.

In my opinion any should be default constructible by default. I think
this is what most user expect, and std::function, and boost::any are
also default constructible. But of course, this is up to you.
> 
>> 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.

The second one alone is not sufficient. But the first one alone is ok.
At least this what the compilers tells me if I try it with some
combinations of any.

> 
>>
>> 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.

Yes, I have a specialization for 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);

ok.