Re: [boost] [function types] Formal Review begins today.

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2005-06-15 15:56:38

Hi Andy,

thank you for your review !

Andy Little wrote:
> "John Maddock"  wrote
> 
>>The Formal Review of the Function Types Library begins today, and runs until
>>16th June 2005.
> 
> 
>>What is your evaluation of the design?
> 
> 
> Not being a target everyday user of this library I wouldnt like to make too much
> comment.. OTOH ... here goes  ;-)
> 
> Is there a proposed use for these tags? I'm interested in the reason for use of
> these tags over separate functions.
> eg
> instead of ..
> 
>     template<typename Tag, typename T>
>     struct is_function_type;
> 
> Why not :
> 
> is_any_function<F>
> is_variadic_function<f>
> 
> etc

The tags allow us to have two coordinated counterparts 'function_type' (builds
function type) and 'function_type_signature' (takes a function_type apart).

The tags may change with the configuration of the library and they are very
lightweight.
Using metafunctions instead would mean generating lots of code that is never
used in most cases: There would have to be two metafunctions per tag - one for
classification and one for synthesis (except for the "abstract" ones).

Looking closer at the type synthesis part we would end up with lots of functions 
that do exactly the same thing. E.g:

     function_pointer<Seq>
     defaultcall_function_pointer<Seq>
     non_variadic_function_pointer<Seq>
     non_variadic_defaultcall_function_pointer<Seq>

or we would only use the "most concrete" (the one at the bottom of the list -- I 
guess its name is already enough reason not to seriously consider this) or we 
would drop the symmetry of sythesis and decomposition (imposing a huge ammount 
of irregularity on the user).

> OTOH why not do it the other way round eg
> 
>     template<typename F, typename Tag = any_function>
>     struct is_function_type;

I found it appealing to have the tag next to ("extend") the identifier.

> eg  giving is_function_type<F> for what I guess is the most usual case.

Don't think so. I don't see a reasonable default, here.

> I know this overlaps type_traits, but therefore is this any_function necessary?

In fact, this particular case doesn't (at least not exactly), but it can be 
modeled with TypeTraits, if this is what you mean. It will look somewhat like 
this (untested freehand code):

     mpl::or_<
       is_function<
         typename remove_reference<typename remove_pointer<T>::type>::type
       >
     , is_member_function_pointer<T> >

> Does providing non functions-types to eg function_type_arity<F> give a compile
> time assertion? Cos I think it should!

It gives you a compile error at the point you try to access '::value'
("function_type_arity<Blah blah blah> does not have a member named value").

'BOOST_STATIC_ASSERT' or 'BOOST_MPL_ASSERT*' are not used for this, because this
would trigger an error inside the library, instead of where the actual problem
in client code is.

IIRC using this technique was inspited by TypeTraits.

>>What is your evaluation of the implementation?
> 
> 
> I havent looked at the implementation.
> 
> 
>>What is your evaluation of the documentation?
> 
> 
> I would really like an accessible example right at the start.
> What is this library useful for? The example source files seem to have little
> regard for anyone reading them.

You're right - it's missing.

> The closure example looks interesting 
> (though I'm not sure what a closure is) but
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
There is a brief introduction in the header, if it doesn't work for you, try
Wikipedia or alike.

> perhaps this could be the basis of an in depth example followed through in the
> documentation. There are few comments in the examples  and in a lot of cases
> they just look like test programs.

There should be something like this. I'm not sure if I want to use one of the 
in-depth examples (they are probably too far out for the docs) or smaller but 
still "real ones", though.

I think per-function examples are important, too. And I can try to come up with 
  some less boring ones...

> I'd especially like to see an example of application to callbacks.

Well, the closure example shows a special case of "callback" (partial argument
binding).

Another, more general example in this direction has been recently added to the 
library:

     - interpreter.hpp
     [ http://tinyurl.com/bu5vn ]

     - interpreter_example.cpp
     [ http://tinyurl.com/8ryt3 ]

> eg (Apologies I may be rambling, but I'm trying to find simple  useage scenarios
> in the following. Of course standard library examples would be better))
> 
> [...]

Let me ramble too and try an STL-style version (and I'll substitute 'remove_if' 
from your example with 'partition' - because I find it just too weird, 
otherwise. It's still odd enough, I guess).

     template<std::size_t Arity> struct sort_somehow_impl;

     // Sorts or partitions the data in the range [from;to) for unary or binary
     // predicate functions respectively
     template<typename Iterator, typename Function>
     void sort_somehow(Iterator const & from, Iterator const & to, Function f)
     {
       sort_somehow_impl<
         ::boost::function_type_arity<Function>::value
       >
       ::work(from,to,f);
     }

     template<> struct sort_somehow_impl<1>
     {
       template<typename Iterator, typename Function>
       static void work(Iterator const & from, Iterator const & to, Function f)
       {
         std::stable_partition(from,to,f);
       }
     };
     template<> struct sort_somehow_impl<2>
     {
       template<typename Iterator, typename Function>
       static void work(Iterator const & from, Iterator const & to, Function f)
       {
         std::stable_sort(from,to,f);
       }
     };

When the function template  'sort_somehow' is instantiated it will generate a 
body that either invokes 'std::stable_partition' or 'std::stable_sort' depending 
on the arity of the passed function pointer or -reference.

Since it's an STL-style example we might want to add support for STL-style functors:

In my reply to Alisdair Meredith's comments I explained why you would need to 
depend on typeof if you wanted to provide an interface similar to the one of 
this library for simple functors.

However, the arity is a special case because it's numeric. We _can_ write a 
generalized metafunction that works on both function types and simple functors:

     // This one knows the type of the member function pointer expression
     // and can return the arity encoded in the size of a char array reference.
     template<typename MemFnPtr>
     char (& simple_functor_arity_impl(MemFnPtr))
     [ ::boost::function_type_arity<MemFnPtr>::value + 1 ];

     // Uses the sizeof operator to decode the arity from the result of the
     // function template declared above when used with the expression
     // '& F::operator()'.
     template<typename F>
     struct simple_functor_arity
       : boost::mpl::size_t
         < sizeof(simple_functor_arity_impl(&F::operator())) - 1 >
     { };

     // Wrapper for 'function_type_arity' which will try to inspect the type
     // of the expression '& F::operator()' instead of F, if F is not a
     // function type.
     template<typename F>
     struct function_or_functor_arity
       : boost::mpl::if_< boost::is_function_type<boost::any_function, F>
                        , boost::function_type_arity<F>
                        , simple_functor_arity<F> >::type
     { };

Now the only thing left to do is replace this line

         ::boost::function_type_arity<Function>::value

from our first attempt with

         ::function_or_functor_arity<Function>::value

and maybe add this comment:

     // ATTENTION: this will not work for function objects with a
     // parentheses operator which is a function template or an
     // overloaded function

to make our example work with simple functors.

Here is a skeleton to reproduce it with a compiler (all code has been tested 
with GCC 3.4):

     #include <cstddef>
     #include <algorithm>
     #include <functional>

     #include <iostream>

     #include <boost/mpl/if.hpp>
     #include <boost/mpl/size_t.hpp>
     #include <boost/function_types/is_function_type.hpp>
     #include <boost/function_types/function_type_arity.hpp>

     // <--- the code discussed above comes here

     int main()
     {
       static const std::size_t n = 10;
       int e[] = { 5,0,2,0,7,0,8,1,12,1 };

       sort_somehow((int*)e, e+n, std::logical_not<int>());

       for (int i = 0; i < n; ++i) std::cout << e[i] << " ";
       std::cout << std::endl;
       // output: 0 0 0 5 2 7 8 1 12 1

       sort_somehow((int*)e, e+n, std::less<int>());

       for (int i = 0; i < n; ++i) std::cout << e[i] << " ";
       std::cout << std::endl;
       // output: 0 0 0 1 1 2 5 7 8 12

       return 0;
     }

>>What is your evaluation of the potential usefulness of the library?
> 
> 
> I think its up to the author to show me with better simpler examples, showing
> this.
> 
> 
>>Did you try to use the library?  With what compiler?  Did you have any
>>problems?
> 
> 
> I tried a couple of simple examples in VC7.1. No problems.
> 
> 
>>How much effort did you put into your evaluation? A glance? A quick reading?
>>In-depth study?
> 
> 
> A couple of hours thinking and reading.
> 
> 
>>Are you knowledgeable about the problem domain?
> 
> 
> No.
> 
> 
>>And finally, every review should answer this question:
>>
>>Do you think the library should be accepted as a Boost library?  Be sure to
>>say
> 
> 
> I  abstain.  If its useful in implementing all the libraries it says it can then
> it must be very useful, but I'm afraid that the author must take more trouble

David Abrahams suggested to apply this library to existing Boost code as a proof 
of concept. I'm currently working on it.

> with explaining things to simpler souls like myself in my opinion.

Writing documentation, especially good documentation understood by a wide 
audience, is not an easy task. I honestly hope I am not too simple of a soul to 
succeed in making the next revision more understandable.

Regards,

Tobias