[boost] [function types] Review

From: Jonathan Turkanis (technews_at_[hidden])
Date: 2005-06-19 01:58:57

Hi,

Here's my review of the function types submission. I'm sorry I waited until the
last minute to submit my review.

I vote to accept the library, even though I have some problems with the design.

> What is your evaluation of the potential usefulness of the library?

The library is potentially very useful. For my interfaces library (
http://www.kangaroologic.com/interfaces/) I needed a facility for constructing
and manipulating function types. Because nothing was available, I wrote this:

http://www.kangaroologic.com/function_traits/libs/function_traits/doc/

However, as Tobias politely pointed out, my implementation was a bit naive. It
would be valuable to have a single correct and efficient implementation which
could be shared by various current and proposed boost libraries.

> What is your evaluation of the design?

My main criticism is that the metafunctions taking tags are given too prominent
a place in the library. I would rather have the library provide a collection
metafunctions to handle the most common combinations of tags and provide the
more generic metafunctions for 'advanced' use.

For example, I'd like to have metafunctions 'is_[plain_]function_type,'
'is_function_pointer,' 'is_function_reference' and
'is_const_member_function_pointer,' like I provided in the above mentioned
library, in addition to 'is_function_type.' Also, I'd like to see specialized
metafunctions for constructing (plain) function types, function pointers and
function references, in addition to the general purpose metafunction
'function_type.'

Next, function_type_signature is too complex; it has too many members, and their
specifications are hard to understand. I would like to see separate
metafunctions which take a function type and return

   1. the sequence of arguments; you might call this function_type::arguments
   2. the sequence of arguments, including ant implicit 'this' pointer; you
might call this function_type::effective_arguments
   3. the sequence (return_type,  [class,]  arg0, ... argN). You could call this
function_type::signature.

Alternatively, the result of instantiating the templates might themselves be
sequences.

Next, it's too hard too keep track of all the different tags, esp. remembering
the order of the components of the names. Why not have a few atomic tags, and
let user's combine them via inheritance? E.g.,

    struct const_tag { };
    struct volatile_tag { };
    struct reference_tag { };
    struct variadic_tag { };
    struct stdcall_tag { };

    struct my_tag :
         reference_tag,
         stdcall_tag,
         ...
       { };

   typedef is_function_type<my_tag, T>::type result;

You could define a bunch of convenience tags for common cases, but people
wouldn't be required to learn them. This is what the iostreams library does.

Finally, I don't like the fact that the term 'function type' is used to refer to
pointers and references. This is bound to cause confusion. Also, it means that
you have to write 'plain' all over the place. In my library, I used the term
'signature type,' which I wasn't too happy with, but at least it was clear that
I wasn't referring to any pre-existing category of types.

> What is your evaluation of the implementation?

I studied an early implementation that didn't define any MPL sequences. It was
good.

> What is your evaluation of the documentation?

The documentation needs major improvement. This has been addressed thoroughly in
other reviews.

> Did you try to use the library?  With what compiler?  Did you have any
> problems?

I used previous versions of the library, but not the current  version.

> How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

I've spent much time considering the problem domain, but only about two hours
during this review period.

> Are you knowledgeable about the problem domain?

Yes.

Jonathan