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

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

Hi Jody,

Jody Hagins wrote:
> First, this looks very cool, and I think it will get a lot of use even
> from those who are a bit scared of full MPL.
> 
> 
> This is not my official review, but simply some questions/comments about
> the documentation, specifically the things I think of as I read it for
> the first time...
> 
> 
> I am not a specialist in grammar, and I have the distinct disadvantage
> of American English as my native language (well, most will say that
> South Carolina-ese is English).  However, I had to read the first
> sentence of the MOTIVATION section three times to decipher its meaning. 
> I think it could be better understood if it were restructured...
> 

OK. All I have to say for my excuse is that English is only my secondary 
language...

So, if the first sentence is hard to understand it should be changed.

> Also, in the first paragraph of MOTIVATION, the author asserts "common
> applications" and "well known special cases" of which a large number of
> people are not familiar.  IMO, if you reference a motivating case, you
> should explain it a bit more, or provide references to examples.
> 

Good point.

> I am similarly confused by the last paragraph of MOTIVATION.  What is
> meant by "It covers the functionality of the function_traits class?"
>

It means that (given the your compiler is supported by the library) all that can 
be done with Boost.TypeTraits' function_traits class (and more) can also be done 
with the FunctionTypes library.

> After reading the OVERVIEW and MOTIVATION, I am still at a loss for what
> benefit this library will provide.  I can make some reasonable
> assumptions, but I would think that I should have a much better picture
> than I currently have from reading those two sections.

What exactly is unclear ?

> 
> Obviously, I am either a total idiot, or I am just having a bad day
> (please do not confirm the former if you know me personally).  I am
> having a hard time groking the docs for CLASSIFICATION, especially the
> summary.  I do not think I should have to read the full description to
> get an idea of what the function does, yet after reading the summary, I
> thought, "Hmmm... maybe a read through the details will help."  To me,
> this indicates a little more information should be in the summary.
> 
> Also, the CLASSIFCATION docs say that it tests to see if a type is an
> element of a set of other types, and gives several .cpp files as
> examples.  However, after reading the entire specification for
> is_function_type, I have these questions...
> 
> How do I build a set of types?  How do I test for a type in a set?  All
> examples show one type, not a set of types.  
> 
> What should happen if I put types in the set that are not function
> types?  Should I expect a compile error, or is it up to the programmer
> to know specifically that the types belong in that set?
> 

Here's a fundamental misunderstanding:

There are no sets of types that can be _built_.

You just _name_ a set of possible kinds of function types to test against by 
specifying a tag:

    is_function_type< function_pointer, T >

Tells me if T is contained in the set of "all function pointers", which is 
described by the tag type function_pointer.

I can ask for a superset (all types handled by the library) like this:

    is_function_type< any_function, T >

Or a subset:

    is_function_type< variadic_function_pointer, T >

Did you read the Tags section at all ? Was it unclear ? Where ?

> 
> For the function calls, the descriptions make some assumptions (like, it
> must be called with a function type).  Are these suggestions for good
> style, or will the implementation provide a compile-time assertion, or
> will you ger undefined behavior if you violate the assumption?
>

If you violate the preconditions the behaviour is generally undefined.

However, the type members won't be defined to allow better diagnostics for 
client code (this is currently undocumented).

> function_type_arity: Should probably put note about BOOST_FT_MAX_ARITY
> here (or reference the CONFIGURATION section) to make it obvious early
> about the max arity.  

BOOST_FT_MAX_ARITY applies to the whole library. Still, a reasonable point.

> What happens if you call it with a non-function? 
> Do we get a compile time assertion?

Another violated precondition. We had this above, already.

> 
> function_type_parameter: should probably say that the index starts at 0
> (this is the common C/C++ method, but stating it explicitly would be
> helpful).  

Agreed.

> It would also be helpful to be a bit more explicit about the
> difference between function_type_parameter and
> function_type_parameter_c, since not everyone groks MPL as second
> nature.

Well, in case you don't know MPL (although the primary audience most likely 
will) you can still have a look at the synopsis and look for the version with 
non-type template parameter...

> How do functors fit in here?  If a class overloads operator(), is it
> considered a function type?

No. Is the definition really that unclear ?

How could this probably work if operator() is a function template ?

You can decompose a member function pointer to a parentheses operator (which 
implies it's already instantiated if it's a template function), though (after 
deduction or using some kind of typeof operator).

> 
> In the ENCAPSULATION section, I think the author is again making too
> many assumptions about the detailed knowledge of MPL.  This library,
> while using the MPL, and imitating some aspects, does not require full

This _part_ of the library provides an MPL Sequence interface. There is really 
no point in using it without knowing the MPL and its algorithms.

Yes, you can use the library without knowing MPL, but this particular part is 
about MPL interoperability. Sorry !

> knowledge of the MPL.  However, the docs seem to take too much for
> granted, and make too many assumptions.  I'd like to see more detail,
> especially in the descriptions.

What do you mean with "assumption" ? Preconditions on template arguments ?

As this library's classes are _slightly_ more complex in it's use than the ones 
of Boost.TypeTraits (in some light it can be seen as an extension) what makes 
you think there has to be so much more detail ?

> After my initial reading, I am still not entirely sure about
> function_type_signature.  I would like to see more description
> information.  Also, I do not like the "types" member.  What is the
> rationale for the ordering of the types (return, [class], parameters)? 
> Callers would have to query even more information to determine how to
> interpret types.

These members are documented at all because they might be helpful in some rather 
rare cases - I should probably remove some of them from the documentation.

The primary interface of function_type_signature<T>, however, is to use it as an 
MPL-Sequence.

> Also, if you remove c-v qualifiers, how can a caller ever get what was
> truly passed to function_type_signature?  Maybe adding something like
> raw_representee, or something else so that the caller can see the true
> T?

Very good point - this is unclear: it refers to top-level cv qualification of 
pointers. E.g:

     void(*const)();

The cv-qualification of "member function pointees" is _never_ dropped.

> 
> function_type: looks very intetresting.  Unfortunately, none of the
> thoughts/questions in my head about usability are answered by the
> examples.
> 
> 
> PORTABILITY: If you have not tested with BCB, maybe you should (or get
> someone to do it).  Or, at least note that it is most likely broken on
> that compiler, w.r.t. function references.  Specifically, BCB6 does not
> properly distinghush...

BCC is broken for all sorts of reasons. It is _not_ supported and does _not_ 
speak enough ISO-C++. Btw. I know a workaround for the problem regarding 
function references but the lack of partial specialization for member function 
pointers is much worse.

I guess Boost.Typeof could be used to work around it (once it supports Borland), 
  but I'm really not sure it's worth putting much energy into supporting this 
compiler (yes, I know BCB is cool for quickly building user interfaces) as the 
product suffers halfhearted maintainance and probably won't be around for very 
much longer.

> 
> int (func)()
> int (*ptr_to_func)()
> int (&ref_to_func)()
> 
> 
> Should the CONFIGURATION section go before the detailed descriptions of
> the function calls?
> 

Should it ?

> 
> In general, I do not like using test assertions as examples.  I know
> lots of boost stuff uses this methodology, but it does not provide much
> information.  If tests are used, then they should include LOTS of
> documentation describing exactly what is (and is not) happening, and
> sufficient justifications.

I never had problems with "assertions as examples" as a user...

> 
> After reading the docs, I think many will be at a loss for concrete
> examples of real-world use for this library.  

It factors out existing Boost functionality in components like Bind, Lambda, 
ResultOf, Phoenix....

I do not want to change around these components (the library authors are free to 
do so of course - and I guess, and hope, some will welcome it), however, there 
are many more specialized situations it would be necessary to duplicate the 
redundancy in these components yet another time.

How about a callback registration that knows from the signature of the 
registered function the data to call it with ?

(This was my reason to need the functionality of this library)

Component writers who use this library also get the benefit of a central point 
of configuration and being better neighbours with other components which need 
the same functionality in terms of compilation speed.

> Again, in a library like
> this, the documentation should include LOTS more examples, and each
> example should be fully documented.  Otherwise, the curve to usage is
> enormous.

How could someone unfamiliar with template metaprogramming (with or without 
using MPL) possibly be interested in using this library ?

So I'm not sure you are among the intended audience. And I really would love to 
hear more voices from those who undoubtfully are, on what _exactly_ needs to be 
improved.

> Finally, I'd like to see some examples that support the motivation.  The
> author cites some reasons for the library to exist.  I want proof, in
> the form of examples that show how difficult (or impossible) it is to do
> what he wants without the library, and then examples showing how the

E.g. let's extract the first parameter of a member function pointer type:

Currently there is no way to do it except use Boost.Preprocessor to create a 
cascade of partial template specializations for every possible arity both with 
and without ellipsis for every calling convention required (or write it by hand, 
if you like ;-) ).

> same problem is easily solved with the library.  I do not think this is
> just unique to this library.  I think it is important for all libs,
> hint... hint...

With the FunctionTypes library the above scenario looks like this:

     function_type_parameter_c<T,2>::type

and it will compile as fast as the hand written version (the library provides 
preprocessed headers - and is probably even faster because plain function types 
and function references are transformed to pointers before using specialization 
to match the type).

Thank you for the huge ammount of feedback !

Regards,

Tobias