Re: [boost] [review] Fast track formal review of Forward library begins today

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2007-12-06 15:36:09

Giovanni,

thanks for your review.

Giovanni Piero Deretta wrote:
> On Dec 3, 2007 10:57 AM, John Torjo <john.groups_at_[hidden]> wrote:
>> Hi all,
>>
>> The formal review of the Boost.Functional/Forward library, proposed by
>> Tobias Schwinger, begins today :
>>
>>      * What is your evaluation of the design?
> 
> Good. Simple enough to cover most needs.
> 
> In my design I allow the user to specify if a specific operator()
> overload should be disabled or not (depending on the arity and the
> parameter types); It is useful if you are using more than  one
> forwarder in some complex composition (i.e. as base classes) and you
> need to disable ambiguous overload. I do not think
> Boost.Functional/Forward should necessarily provide this
> functionality, but it might be considered.

Yes, we have been there (with Fusion) and recently dropped this
functionality when porting to Boost.ResultOf:

We used to have an empty 'result' metafunction say "disable this
overload", exploiting SFINAE where applicable.
ResultOf, however, does not propagate emptiness by specification as a
matter of downward compatibility (no SFINAE inside the operand of 
'decltype').

I personally still have mixed emotions about the loss.

> 
> I think too that the library should optionally allow K more arguments
> which are all const or non const. For (uncommon) very high arity
> functions that need to be wrapped, you can usually live with this
> limitation.
> 
>>      * What is your evaluation of the implementation?
> 
> Looks simple, but it is hard to evaluate preprocessor meta code.
> 
>>      * What is your evaluation of the documentation?
> 
> Complete enough.
> 
> I think it should be explicitly specified that the wrapped function
> object must be result_of
> compatible (if such a note is already present, I've missed it).

OK, here is some overlap with Joel's complaints. It seems I should add 
some detail in regard to result_of usage.

> 
> A discussion of the impact of the library on compile time (especially
> with high values of N) would be useful.

Yes, I think the final version will have a it and give the user more 
control over the N (see my reply to Joel's review).

> 
> Also a comment on the (potentially lack of) runtime performance
> penalty imposed by the library would be nice.
> 

That would be "the lack thereof with a decent compiler". That is there 
are no constructs that force the compiler to emit code - though it's 
still free to do so.

>>From the documentation I cannot infer if operator() is overloaded for
> both const and non const 'this'. I think not (), but it should be
> specified explicitly.  

Count it as a bug.

> I have never needed this in practice, In fact,
> IMHO it should be specified that 'this' is always const.

Considerable, but I believe I want to think some more about it :-).

Regards,
Tobias