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

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

Jody Hagins wrote:
> On Mon, 06 Jun 2005 22:37:06 +0200
> Tobias Schwinger <tschwinger_at_[hidden]> wrote:
> 
> 
>>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.
> 
> 
> That is a much better description.
> 

Seems like we have a fundamentally different taste when it comes to documentation...

I find the documentation too long already, for that matter.

> 
>>>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 ?
> 
> 
> Well, please understand that when I read stuff as a "reviewer" I do so
> in a state of ignorance (some may say I live ther, but that is another
> story).  I do not think the overview and motivation sufficiently

Are you sure you're not overdoing it ?

> describe the problems that the library is trying to solve.  I think too
> many assumptions about the reader's expertise are made.  The library
> itself is relatively easy to use, and does not require a lot of
> experience.  However, I think the descriptions are not sufficient to
> show readers why they should use the library, and what problems it
> solves.

I just re-read the 'Overview' section trying to imagine I would not know much 
about it and it still gives me a clear impression of what the library does.
The only thing I see right away is there could be some examples for which 
properties to inspect.

I agree the 'Motivation' section can be improved and I will consider your points 
once I find the time to rewrite it.

> 
>>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 ?
> 
> 
> Yes, I read it, and I understand it.  However, the documentation is a
> bit unclear, as it implies a set of types, not one specific tag
> identifier.  I'm not trying to be difficult, but I am trying to find
> places that need improvement.  In this case, "if a given type is element
> of a set" along with the description of Tag, where any of the tag types
> are allowed, can cause confusion because a set implies one or more
> items.  I would suggest changing the documentation from using the term
> "set" to being more explicit about using one of the tag types.

Does perhaps it help to say "infinite sets" ?
Or even "infinite immutable sets" ?

Every occurence of "set of types" is almost immediately followed by "specified 
by one of the tag types" or alike, where "tag types" is a hyperlink to the 
description.

> Again, however, this is just the opinion of one person.  Take it for
> what it's worth.

Well, I think it takes more than two people's opinion (with very different 
tastes and one of them being the author) to get a clear picture how to get the 
documentation right.

However, I do appreciate your feedback. And (to cover another paragraph further 
below and the follow-up) do _not_ take this as a personal offense or whatever. 
It's valuable (even I do not agree in every point and just reply to some of 
them: I have to think about it one more time and it clearifies the rationale 
behind things, which is sometimes based on intuition)...

> 
>>>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.
> 
> 
> Hmmm.  I would prefer a compile time assertion inside the functions, if
> possible.
> 

...like this one ;-). Actually my first version used static assertions...

> 
>>However, the type members won't be defined to allow better diagnostics
>>for  client code (this is currently undocumented).
> 
> 
> I do not understand the above sentence.
> 

This version doesn't use static assertions but e.g.

     function_type_arity<int>  has no member named 'type' or 'value'

This tells the user that function_type_arity sees a wrong argument.

The line number of the error message is within the user's code (where it 
happens) and not somewhere within the Function Types library (as it would be 
using static assert, which further stops the compilation process in some contexts).

In fact, I did not invent this technique, it can be said to be common practice...

>>>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...
> 
> 
> I would disagree with you here.  Many boost users do not know MPL.  Many
> more probably have a cursory knowledge of MPL, but still need
> documentation handy to make use of it.
> 

I believe you did not get my point, here:

I'm trying to say that a user who does not know MPL would not know type to pass 
for an "Index" and thus use the version expecting a non-type template parameter 
of type std::size_t instead.
This does not assume MPL knowledge, just a bit of intelligence ;-).

> 
>>>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).
> 
> 
> No, it is clear, but I would think that some notes about operator(),
> boost::function, and boost::lambda would be appropriate, since some
> folks may want to use them in some ways related to function_types.
> 

Is there a note in the TypeTraits documentation that remove_pointer cannot be 
used to retrieve the pointee type of a smart pointer ?

No, because it's clear that it is a low-level facility true to the very nature 
of the type system (in fact, making remove_pointer do the above thing would 
severaly screw up things).

As mentioned in my previous post, components such as Lambda, Function and Bind 
(or in some ways similar but perhaps more specialized) components are potential 
clients of this library.

I am having difficulties to come up with good use cases here (and these 
components do not have that much to do with each other from a user's 
perspective, except that they all deal with functions in one way or the other):

The parentheses-operator of Lambda-functors is a function template (so I really 
can't see a useful application in this direction).

It's rather trivial to use it with boost::function (assuming the "preferred 
syntax") via partial specialization, e.g:

     template< typename Function, std::size_t Index >
     struct func_param
        : function_type_parameter_c<Function,Index>
     { };

     template< typename Sig, std::size_t Index >
     struct func_param< function<Sig>, Index >
        : function_type_parameter_c<Sig,Index>
     { };

to do e.g. concept checking:

     template< /*...*/ typename CompareFunction >
     void my_algorithm( /*...*/ CompareFunction compare )
     {
       BOOST_MPL_ASSERT((
         is_same< typename func_param<CompareFunction,0>::type
                , typename func_param<CompareFunction,1>::type >
       ));
       // ...
     }

I'm not convinced of this is of any use either, though.

Using a pointer to parentheses-operator (on functors where it is not a template 
function) probably gives a nice example.

> 
> 
>>>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 ?
> 
> 
> I think interface documentation should make the lowest common
> assumptions.  Not everyone is familiar with the types of problems this
> library is trying to solve.  I am sure you are not merely targeting an
> audience of people who could implement this library themselves, but
> those who would like this functionality without implementing the
> details.

Someone who doesn't know a problem is not looking to solve it...

> 
> FWIW, most boost libraries fail in this regard, in my opinion.  Library
> documentation should make as few assumptions about the knowledge and
> experience of the user as possible.
>

Well, I disagree here:

Is the decision whether or not to use a software components really a matter of 
being able to implement it ?

I believe this is a very onesided view: Given enough time, pretty much 
everything is implementable and finding an elegant and suitable design to 
encapsulate things can be much harder.

Everyone knows the differences of file systems on different platforms and can 
come up with a half-baked solution to make things portable and I guess everyone 
who uses lexical_cast knows how to convert integers to strings without it.

So why are we using generic components at all ?

Because it simplifies our code and makes it more expressive, probably adds to 
its runtime efficiency, because we can subscribe to the design and because it's 
a lot of work to get things right out on our own and because the libraries 
handle subtle problems we would only become aware of when implementing things 
ourselves.

> 
> 
>>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 ?
> 
> 
> Because the current detail is not sufficient ;-)  Seriously, I think
> this library is very much in the reach of a boost newbie to use. 
> However, the documentation and examples are written for a more
> experienced audience.  Thus, I do not think the documentation fits a
> significant segment of potential users.  If you do not intend to target
> relatively new users to boost, then I guess you can leave it alone.
>

Well, I do - but it's probably not the very best starting point to learn about 
template metaprogramming (and that's why there are so many links to other places 
in the boost documenation).

Only some parts of the interface, however, assume you are familiar with MPL. And 
you don't have to read up the concepts of MPL to use most of the library. It's 
helpful to know in case it is used in conjunction with MPL, though.

> 
>>>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.
> 
> 
> I like them there, but they do introduce questions about usage.  In
> particular, you need to leave "kind" because as far as I see, it is the
> only way to get the tag identity of a signature.

You should not use it directly in 99% of all cases and use

     is_function_kind<Tag, T>

instead. Actually there are no public functions to properly compare the tags so 
you should stay away from this.

> 
> I question the "types" because I think it could be a little easier to
> get the return type, and the parameters without the optional class
> sandwiched inbetween.
> 

Actually it _is_ a lot easier:

     function_type_signature<T>

is a model of MPL Sequence.

No need to mess with the 'types' member, unless for optimization (e.g. you want 
a mutable sequence and don't want dependencies to function_type.hpp).

> 
>>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.
> 
> 
> Fine with me.  I've been advocating dropping compilers that are too
> broken, or require too many workarounds, for a while now.  However, BCB
> is used by a number of boosters, and until Borland comes out with
> something else, they are a bit stuck.  Maybe an explicit note about BCB
> would be a good addition to that section.
> 

Well, BCB is not listed among the supported compilers - this should be enough IMO.

I plan on applying some broken compiler workarounds although I am not too sure 
which ones, yet. I hope this review will clearify the requirements.

AFAIK things are not likely to get much better with BCC and it will be around 
for some more time (even though Borland stops the development entirely) - a good 
reason to provide (at least limited) support for it.

> 
> 
>>>Should the CONFIGURATION section go before the detailed descriptions
>>>of the function calls?
>>>
>>
>>Should it ?
> 
> 
> Dunno.  I had questions about the calling convention and arity long
> before I got to the section called CONFIGURATION.  It was not obvious at
> all when I looked at the table of contents to the left that I should go
> to the CONFIGURATION section to answer those questions.
>  

Good point.

> 
>>>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...
> 
> 
> Good for you.  I doubt that is the case for everyone.  Most of these
> tests are unit tests, which do not necessarily show typical use cases. 
> In my opinion, I'd like to see typical real-life use cases as examples. 
> In particular, something that solves the main reasons for the library
> existence, so I can see true ways of using the library.
> 

The problem is that most real-world use cases do involve more of the library 
than just a single function, so it'll be harder to understand what a particular 
function does, this way.

> 
> 
>>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)
> 
> 
> This seems like a reasonable example, and will probably use most of the
> interface as well.
> 

I'll try to add more real-world examples.

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

Well this should sound: "how could someone unfamiliar with network programming 
be interested in a socket library ?"...

> signals/slots possibly be interested in the signals library?  I use a
> number of boost libs on a daily basis.  However, I did not start using
> them because I was already an expert in that domain.  Primarily, I
> started looking at some documentation, and it then hit me how I could
> use the lib to solve some of my own problems (or later, when a problem
> came up, I rememebered something I read about a lib). 
 >
> I think you are approaching the documentation from the standpoint of
> someone who already knows the details of implementing and using the
> library, and not from the POV of someone who does not even know if they
> should use the library.  I think we should strive to meet users where
> they are, and through documentation, bring them to the level of the
> developer.
>

...however I get your point. I have to think about how to improve this.

> 
> 
>>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.
> 
> 
> Don't be so sure ;->  I'm not exactly the novice for which you seem to
> take me.  However, I do believe that documentation should be written in
> that vein (and I also believe documentation should be reviewed in the
> same vein).

In fact, I don't think a pathological interest in torturing compilers is a must 
to be a good programmer. It leads to some interesting techniques regarding both 
design and implementation and because of this boost folks appreciate these (at 
first glance) tricky techniques.

This said, I just mentioned some doubt if the novice you are (or not are, but 
pretending to be for the purpose of bulletproofing the documentation) has a need 
for this library. No intentional offense, here.

<snip>

>>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).
> 
> 
> Actually, this is a great example, but note that it will bring up more
> questions (e.g., why 2 and not 1? 

Because it was late in my timezone yesterday and I tend to turn into an idiot 
when tired ;-):

It should be 0 (the first parameter) not 2 (the third parameter) !

> and how would I do it if I did not
> know it was a member-function-ptr type?  

The same way.

> -- which is the basis for my
> question about "types" since it requires some knowledge about the
> optional class element of the sequence).

When using 'function_type_signature' (or its 'types' member, which, as stated 
above, is not recommended) it does make a difference (because these are 
sequences containing all sub-types not just the parameter types).

In case you need an MPL Sequence of all parameter types there is the convenience 
function 'function_type_parameters' (plural).

> 
> I'm really looking forward to the callback registration example.  I
> think it is similar to something I wrote to help automate registration
> of slots with the appropriate signals (though probably more general).
>

I hope I'll get it done in the next couple of days.

> 
> 
>>Thank you for the huge ammount of feedback !
> 
> 
> You bet.  I only hope some of it turns out to be valueable, and not just
> "reply-fodder."
> 

As stated above, even "reply-fodder" helps in enhancing things. And yes there 
are valuable points among it.

> Thanks again for your submission, and desire to enhance open source
> libraries.
> 

You're welcome.

Regards,

Tobias