Re: [boost] [function_types]

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2006-11-17 05:50:35

Hi Steven,

Steven Watanabe wrote:
> AMDG
> 
> I'm not sure whether function_types is ready now or not.
> I don't really like the tags. 

Hmm... If it isn't there might be something wrong with how we review things:

<cite>
  [...] The function types library is accepted into Boost, subject to a further 
  mini-review to be conducted at a later time convenient to Tobias.  
  The "mini review" will be conducted independently to the full review process; 
  it's purpose will not be to reopen issues already addressed at this review, 
  but to give reviewers a chance to:

  * Check the reorganised/renamed classes: [...]
  * Check the revised documentation.  [...]
</cite>

The previous version was pretty tag-heavy. I put a lot of work into reducing the tag-based interface down to a minimum. Please note that there is no need to touch the tag types for a vast majority of the library's use cases. Now folks complain more about the tags than before. Worse than that, I haven't been presented a single clearly superior alternative!

> Dividing the parameters
> into two groups (the type arguments and everything
> else) doesn't seem like the cleanest solution. Even
> with them it will sometimes be necessary to and_<>
> several predicates together
>   mpl::and_<is_function<F>, mpl::equal_to<arity<F>, mpl::int_<1> > >
> so I think making certain properties should not
> be treated specially.

The objective of the tags is to give some (less often required) properties an encapsulation as a type -- not to provide syntactic sugar for nicer queries. In fact, typical use cases will not require that a tag argument is ever given *by design*.

Further, the tags are grouped by properties. Maybe the documentation does not reflect the grouping sufficiently, but there is no such thing like an "anything else group" ;-). 

> I think that components should probably integrate
> all the properties a little better.  Instead of making
> it the union of a tag and an mpl sequence would it
> be better to make it more like mpl metaobjects
> with appropriate mutators:
>   typedef components<void(int, char)> c;
>   typedef set_result<c, bool>::type c1
>   typedef set_param<c1, 0, const int&>::type c2;
>   typedef make_variadic<c2>::type c3;
>   typedef get<c3>::type f; // is bool(const int&, char, ...)
> (Although maybe set_param is asking too much since I don't
> see the corresponding operation in mpl.)
> and querying metafunctions:
>   typedef is_variadic<c>::type b;  //inherits from mpl::false_
> These could be overloaded to take callable builtins too,
> but that might cause too much overhead translating
> back and forth between two representations.

It seems a bit arguable to me whether we're talking about sugar or clutter, here.

> For both components and the synthesis metafunctions,
> having the result type as the first element of the sequence
> doesn't really make sense to me.  I would expect that
> the return type would often need to be handled separately
> from the parameters in the cases where anything other than
> simply passing the result of components to function_type<...>
> is needed.

I found it preferable to have a one-type representation, but I'm currently not feeling to strong about it anymore. Your proposal might be a good idea -- I'll think about it.

Here is one scenario where the current design seems quite helpful, though:

    http://tinyurl.com/whkmd (message thread on g.c.l.boost.user)

> 
> I would like a way to find out what
> a tag is composed of. I saw represents<> in the code
> but it isn't documented (unless I missed it)

Unlike in the previous version, and as stated above, the tags intentionally play a very subordinate role in the interface, so I left out 'represents' and 'extract' for the lack of real world use cases.

> I compiled the following on all the compilers I have with different 
> values for N_TYPES.

> note that this only uses one partial specialization

It doesn't matter much! The compiler has to instantiate over 3000 templates for N_TYPES == 40. During template instantiation the compiler has to check which partial specialization definition matches. You are using over 3000 distinct types so it will have to find out over 3000 times (even if it ends up picking the same definition, over and over again), looking at all specializations there are, of course.

<snip code & benchmarks>

I'm really glad to see that things compile with Codewarrior, since I don't have this compiler and thus never tested the library with it ;-)

> A few minor things.
> pp_tags/master.hpp
>     BOOST_STATIC_CONSTANT(bits_t, combined_bits =
>       LHS_bits ^ RHS_bits ^ (LHS_bits & RHS_mask)
>     );
> works but seems obscure to me
> would (LHS_bits & ~RHS_mask) | RHS_bits be clearer?

Right. It's an artefact and used to be an optimization back when it read mpl::bitxor...

> Here are the mistakes I made when I looked at the
> documentation initially. If no one else runs into these don't
> worry about it. It's probably just me.
> 
> I thought ClassTransform applied to all the parameters
> not just this.
> 
> I didn't realize that components was a tag. I saw the capitalized and linked
> MPL <../../../../../mpl/index.html> - Front 
> <../../../../../mpl/doc/refmanual/front-extensible-sequence.html> / Back 
> <../../../../../mpl/doc/refmanual/back-extensible-sequence.html>Extensible 
> <../../../../../mpl/doc/refmanual/extensible-sequence.html>Random Access 
> Sequence <../../../../../mpl/doc/refmanual/random-access-sequence.html> 
> of all component types
> and missed the trailing "... and property tag "

This one got confused already, the documentation needs to be fixed at this point.

Thank you for your review.

Regards,

Tobias