Re: [boost] [function types][review] (repost with new formatting)

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2005-06-14 13:54:14

David Abrahams wrote:
> Tobias Schwinger <tschwinger_at_[hidden]> writes:

>>is_function_type<Tag,T> is an MPL Integral Constant. 
> 
> 
> You don't say that anywhere.
> 

Well, that's what the "member '...' notation" was supposed to mean:

     ...    -    See the MPL Integral Constant concept.

>>>> an element of the set of types specified by Tag. Member function
>>>> pointers may be more const-volatile qualified than specified,
>>>
>>>
>>>Hmm, have you looked carefully at the use cases for this?  Member
>>>functions often have counterintuitive is-a relationships.  For
>>>example, a base class member pointer is-a derived class member
>>>pointer, but not vice-versa.  It's not obvious to me that any is-a
>>>relationships should hold here.  What are the use cases?

I'll have to add an annotation here: I am looking at this from the perspective
of pointer <-> object comparison: More CV qualification of the member function
pointer is always OK, as it is OK to call a const member function on a non-const
object. You need a pointer to an at least const qualified member function if the
object is const.

>>I'm thinking about removing cv indication from the tags and cv-qualifying the 
>>class type, instead (see comments in the examples interpreter.hpp : line 99 and 
>>function_closure.hpp : line 120).
> 
> 
> Not gonna look at the source, sorry.  No time.

Never mind. It refers to a piece of code (to be more specific to a comment in
front of one), which we'll get rid of in many places, when using a qualified
class type to describe cv qualification of member function pointers:

   // Metafunction to decorate class type with cv-qualifiers.
   // ?! Should this perhaps be the default behaviour of function_type_class ?!
   template< typename MemFunPtr > struct class_of //...

I believe we basically agree on this point (however I'm not sure about the
reference part of your suggestion in this direction - see below).

>>>> Members
>>>>
>>>> value	- 	Static constant of type size_t, evaluating to the number
>>>> of parameters taken by the type of function T describes. The hidden
>>>> this-parameter of member functions is never counted. 
>>>
>>>
>>>That particular behavior does not match the needs of any use cases
>>>I've seen.  I always want to treat the hidden this parameter as the
>>>function's first parameter (actually as a reference to the class, with
>>>cv-qualification given by that of the member function itself).  What's
>>>the rationale?
>>
>>The rationale is that there is no unified call syntax for member function 
>>pointers and non-member/static function pointers, anyway:
> 
> 
> Yes, but I typically build something that *does* have a common call
> syntax when wrapping member functions, i.e. I build a function object.
> I would prefer if the metafunction would reflect its signature rather
> than forcing me to assemble it from this nonuniform blob.
>

It seems to me you are requesting to optimize a generic library for your
favourite, specific use case. And honestly, I'm not even convinced this very
case will end up more optimal. Let me try to show you why:

So you are basically talking about an implementation of boost::mem_fn. Let's
start with this passage of a hypothetical and naive implementation:

     // ...

     template<typename MemFunPtr>
     struct mem_fn_wrap2
       : mem_fn_base<MemFunPtr>
     //  ^^^ holds protected member MemFunPtr val_mem_fun_ptr
     {
       typename function_type_result<MemFunPtr>::type
       operator() ( typename function_type_class<MemFunPtr>::type & c
                  , typename function_type_parameter_c<MemFunPtr,0>::type p0
                  , typename function_type_parameter_c<MemFunPtr,1>::type p1 )
       {
         return (c.*this->val_mem_fun_ptr)(p0,01);
       }
    };

1. This is inefficient, we would usually want to optimize forwarding by
substituting by-value parameters with const references except for scalar types 
(like char, int, float, pointers).

2. Now let's assume we "unify" the parameters: our class type becomes the 0th
parameter

3. The above requires we use a reference for the class type so it can pass
through the metafunction that implements forward optimization (without an effect
but also without having to handle a special case)

4. Now we want to add an overload that takes a pointer to the context object
instead of a reference

^^^^^^ ((<<BOOM>>))

A slight change of our scenario turns our code into a cryptic mess (especially 
when using a systematic way to generate it - i.e. Boost.Preprocessor).

=> This usually indicates our design is neither generic nor too well-chosen.
=> Conclusion: Class types _are_ special. The design should reflect this.

>>Typically we use a cascade of template specializations that does the actual 
>>invocation for different function arities.
> 
> 
> Whaa?
> 
> I don't have any "cascading" template specializations in Boost.Python
> AFAIK.  I don't think Boost.Bind does either.
> 

Sorry for not being very precise, here.

If it's not template specialization it's overloading or numbered functions or
functions in numbered templates or classes (hope I got them all, now)...

>>Only counting "real" parameters here allows us to build such a cascade for 
>>arities ranging from zero to "OUR_MAX_ARITY_LIMIT" without having to deal with 
>>the two special cases that there are no nullary member functions and that we 
>>need to go up to OUR_ARITY_LIMIT+1 for member function invocations if the 
>>context reference is taken into account.
> 
> 
> Okay, I've dealt with that issue.  But it's a (really) minor one.
> Generating these things with the preprocessor should usually be done
> with vertical repetition (you'll get lousy compile times and no
> debuggability otherwise), which makes that sort of iteration bounds
> adjustment trivial.
> 

This sounds like using two (slow preprocessor-) loops in client code for members
and non-members, where using one would be appropriate...

I agree it can be "minor", but I currently fail to see it's "less minor" than
your complaint in the first place.

I'm having some trouble understanding the second part of the above paragraph.
What could "otherwise" possibly refer to in this context ? And isn't vertical
repetition the slowest form of PP repetition there is ? Can you perhaps help me 
with it ?

>>Think about generating the (hand-written) cascade in
>>
>>     libs/function_types/example/interpreter.hpp : line 299
>>
>>with Boost.Preprocessor, for example.
> 
> 
> Sorry, no time to look at the code.  But anyway, I've been there.
> It's easy.
> 

OK. The point was that you can get away with one single and uniform, vertical
repetition for both member- and freestanding/static functions. E.g:

     template<size_t Arity> struct invoker;
     template<> struct invoker<0>
     {
        // invoker function for functions
        // invoker function for member functions
     };
     template<> struct invoker<1>
     {
        // invoker function for functions
        // invoker function for member functions
     };
     //...
     template<> struct invoke< OUR_MAX_ARITY > ...

(This is a simplified pseudo-code version of what I was referring to.)

>>Even if completely binding a function (which I believe is a rather
>>seldom use case), we still need a distinction between a member and
>>non-member function pointer which won't go away no matter how we put
>>it.
> 
> 
> I agree with that but don't see the relevance.
> 

The relevance is, that the member function pointer will need special treatment
anyway, sooner or later. We should also leave it its special properties (like 
habing a class type).
There is no point in providing (an oversimplified, as shown above, for that 
matter) simplification to treat member function pointers and other function 
types more similar - because they are not similar!

>>Further we can still take the size of function_type_signature (well,
>>in this case the result type is counted as well).
> 
> 
> ?? You mean sizeof()?  or mpl::size<>?
> 

mpl::size

>>And parameters indices should be consistent with the function arity.
> 
> 
> I agree with that, but don't see the relevance.  In my world, the arity of 
> 
>   int (foo::*)(int)
> 
> is 2.
> 

You can look at it this way and I do not doubt it's a useful model for several
cases. However, I think it's not very true to the nature of things, because:

1. Noone should put functions into classes arbitrarily. The context reference
refers to function's primary working environment which is semantically different
from its parameters.

2. As mentioned before, there is a syntactical separation in the invocation.

3. The context reference is often passed in a CPU register instead of the stack.
So there is even a technical separation.

>>Further it makes client code more expressive.
> 
> 
> On what do you base that claim?
> 

The separation between parameters and context reference is part of the language
so the average programmer will think of a parameter as being something declared
within a comma-separated list in parentheses following the function's name.

>>I guess it's your turn here to prove my design is faulty ;-).
> 
> 
> It's not so much faulty as needlessly irregular.  I believe that will
> be an inconvenience in some real applications, and at the very least
> will cost more template instantiations than necessary.

For maximum efficiency (== minimum number of template instantiations) there is:

    function_type_signature<T>::types

>>If you can convince me, I'll happily change things, of course.
> 
> 
> How'd I do?
> 

I was trying to say that I'm not pedantic about this issue, but:

     Rationale: "It's so, because it's Dave's taste"

is too weak, IMO ;-).

But let's not argue too much on this and suspend this discussion until after the
proof - probably I see things differently, then. Like the idea ?

> [...]
> 
> But the latter doesn't look like a "use" of function_type_signature at
> all! Anyway, don't clarify your meaning for mehere; propose a
> documentation fix.  Just look very carefully at the words you used
> (like "use," "primary interface," "white box," etc.), and consider how
> they might be (mis)interpreted by someone who doesn't already know
> what you're talking about.
> 

It should state that 'function_type_signature' is used to implement all the
other inspection components (it is very important to give the reader this 
insight, I figure).

The latter should be preferred (and thus recommended) in general, because they
make client code more readable and provide error checking.

The possibility to make direct use the type members of 'function_type_signature'
is mainly for optimization and for getting the represented type of a modified
sequence.

^^ Not literally for the docs, but it's about what this paragraph should say. 
Hope no fuzzy terms are left.

>>> [... function_type_signature ]
>>>
>>>So here you are using the form I like, where class types are treated
>>>just like any other (should be a reference, though).  Why not do this
>>>uniformly.
>>>
>>
>>See above.
> 
> 
> Still waiting for a satisfactory answer.  Seems to me that your
> library only gets easier to use (and learn!) if it traffics in one
> uniform structure.
> 

Oh - looks like our favourite (== only?) disagreement again:

Any template, except 'function_type_signature' and 'function_type' should be
intuitive and straightforward enough to hardly ever require a reader of client
code to look into the documentation of this library.

'function_type_parameter*' reflects its paramter list, 'function_type_signature' 
reflects its signature (where the definition of what's part of the signature 
should be in the docs). That hard?

> [...] 
> 
> Well, I suggest you say something that the average dummy is more
> likely to understand, like, "you better #include <whatever> or this
> won't compile."

;-). Probably a bit too informal to copy it literally, but I like the direction.

>>  [...]
>>
>>It used to be a bit more understandable before I removed a rather
>>strange feature to grab the signature from a class template's
>>parameter list.
> 
> I find it hard to believe that it was easier to understand when there
> was an *additional* feature in its behavior!
> 

The other was its counterpart (in a different template)...

>>I will just remove this as well.
> 
> That sounds like it goes in the right direction.

;-)

>>>Also, special-case "if" clauses, especially those that test whether
>>>something matches a concept (like "is_sequence") tend to destroy
>>>genericity.
>>>
>>
>>Would you mind explaining this in some more detail?
> 
> 
> Take boost::variant, which (at one time -- still?) would accept up to
> N arguments describing the held types, *OR* an MPL sequence of the
> types.  The problem happens when your generic code wants to create a
> variant of one element that happens to be an MPL sequence type.  Now

OK - understood.

> you need a special case that sticks that sequence in another
> sequence.  Well, in that case the sequence interface is the only one
> of any use to you, isn't it?
> 
> The point is that switching on a type's properties often introduces
> nonuniformity of semantics, which hurts genericity.  I didn't analyze
> your case to see if it was a problem here in particular.
> 

Not really, as the properties "MPL-Sequence" and "function type" should be
mutually exclusive.

Well, you can use any type which is not a function type, but it won't make sense
  (and will result in a compile error) - if this type happens to be a sequence
it will "work" again. This should be acceptable as long as this behaviour is
documented, I guess.

> - Improve uniformity, if I can get it.  I would probably accept the
>   lib without that change, but I fear I will grumble every time I use
>   it.

Well, let's see... We should be careful, here.

> 
>>- change docs
>>
>>I hope (but am not entirely sure) it's all doable within the review
>>period.  Any hints on a preferred prioritization?
> 
> 
>   1. Proof it
>   2. Post updated docs that include the proposed naming changes
>   3. Change the code.
> 
> But I'm not too particular about it.  I think the proof should come
> first because I'd happily accept assurances in place of steps 2 and 3
> happening before the review period ends.

Thanks again for your help.

Regards,

Tobias