Re: [boost] Function Types Review Begins Monday, November 6, 2006

From: Richard Smith (richard_at_[hidden])
Date: 2006-11-10 08:00:50

Hi Tobias,

Thanks for your response.   There are a few additional
comments below.

Tobias Schwinger wrote:

> Richard Smith wrote:
>
> > 2.  Use of Tags
> >
> >   After a second glance, it's clear that one reason for
> >   having separate namespaces is to avoid name collisions --
> >   we already have a boost::is_function, for example.  And
> >   this leaves me wondering how boost::is_function differs
> >   from bft::is_function as the names fail to convey the
> >   distinction.  So far as I can see, the additional Tag
> >   template parameter is the only difference, and only three
> >   of the possible property tags are meaningful: variadic,
> >   non_variadic and default_cc.
>
> No. In fact is_function is a candidate to be put in namespace boost.
> The tag parameter has a default, so boost::is_function and bft::is_function are compatible.

Won't this cause problems with Boost.TR1 which simply pulls
in boost::is_function?

  namespace std { namespace tr1 {
    using ::boost::is_function;
  } }

I can't see anything in TR1 that specifically allows
implentations to add additional template parameters to the
<type_traits> metafunctions.  (TR1:4.1/1 says it is still a
UnaryTypeTrait, but that's a different issue.)

Nothing in section 17.4.4 of the C++ Standard (which I
assume applies to TR1) allows implementations to add
template parameters and it isn't allowed under the 'as if'
rule either (see DR94).

Plus, it'll break my code ;-(   I've got at least one piece
of code that takes a type trait as a template template
parameter.

> >   [...] instead of writing
> >
> >     bft::is_function< T, bft::variadic >::value
> >
> >   you would write
> >
> >     boost:is_function<T>::value && boost::is_variadic<T>::value
>
> I think, I don't like it. It's too irregular for my taste.

I'm not sure what exactly you mean by 'irregular', but I
take it that

  boost::mpl::and_< boost::is_function<T>,
                    boost::is_variadic<T> >::value

is no better?

To me, it's a question of diminishing returns -- where the
metafunctions are likely to commonly required it makes sense
for a library to provide convenience interfaces that bundle
everything together; but when they're less common, they can
be combined manually by the user.  The question is where
that distinction occurs.

> > 3.  Extensibility
>
> All you have to do is set the macros BOOST_FT_CC_NAMES and BOOST_FT_CC_<name>
> appropriately (see Reference->Macros).

OK.  I hadn't noticed that.  I like that.

Incidentally, are there any compilers (I'm not aware of any)
that specify a calling convention other than with the syntax

  void (calling_convention *)();

... for example, does anything use

  void (*)() calling_convention;

or

  calling_convention void (*)();

And if so, can the library handle it?

> >   If a tag-like syntax is considered desireable, wouldn't
> >   something like
> >
> >     template < typename T,
> >                template <typename> class UnaryTypeTrait >
> >     struct is_function
> >
> >   be more extensible?
>
> [...] it's supposed to work with MPL-Lambda Expression,
> which do not allow template template parameters.

OK, that's a serious problem.  But MPL lambda expressions
can provide an answer too -- make the second, tag parameter
a lambda expression:

  is_function< T, is_scalar<_> >

> > 4.  Naming
>
> For the default configuration all calling convention tags currently have a _cc suffix.
> But we might as well s/_cc/_calling_convention/g.

That would have my vote, but it's a minor point.

> > 5.  Extern "C" linkage
>
> <snip>
>
> > (mostly because you can't put extern "C"
> >   within a template or atemplate within an extern "C"
> >   block).
>
> Exactly.

The is_union type trait doesn't really work, but it still
exists.

I think it would be good if the documentation said that
extern "C" functions (at least in those compilers that
correctly fold language linkage into the type) *shouldn't*
be matched by default_cc, but that without some as yet
unspecified compiler support, they will incorrectly match.

Likewise I think an extern_c_cc that (currently) never
matches would be good.

> > 6.  What is a 'callable builtin'?
> >
> >   So far as I can see, the C++ Standard does not define
> >   'callable', so I asume that the pertinent definition
> >   is the one in TR1, 3.1/6:
> >
> >   | A /callable type/ is a pointer to function, a pointer to
> >   | member function, a pointer to member data, or a class
> >   | type whose objects can appear immediately to the left of
> >   | a function call operator.
> >
> >   By this definition, function types are not callable, nor
> >   are references to functions, bizarre as this may seem.
> >
>
> <snip>
>
> >
> >   Though I'm not wholly convinced with TR1's definition of
> >   'callable', this is as close as it gets to a standard
> >   definition, and it would seem wise to make pointers to
> >   member data return true.

What are your thoughts on adding support for pointers to
member data to the library?  I guess it depends to what
extra you see this as an extension to the Boost.TypeTraits
library for manipulating function types, and to what extent
it is viewed as a utility component for use in implementing
function wrappers and so on.

> > 8.  'Tag Types' documentation
> >
> >   In several places, the 'Tag Types' documentation
> >   incorrectly gives types two leading underscores.
>
> Where?!

In the non_cv reference:

| Equivalent to __tag<__non_const,__non_volatile>, but
| involves fewer template instantiations when evaluated.

Also in const_non_volatile, volatile_non_const and
cv_qualfied.

Actually, is the documentation in the zip file up to
date?  It has [last-revision $Date: 2005/05/21 13:27:00 $].

> > 10.  Syntactic sugar around parameter_types
> >
> >   Whilst being able to leverage the full power of the MPL
> >   with parameter_types is definitely desireable, it can also
> >   faze those less familiar with metaprogramming.
> >
> >   I would like to suggest a helper metafunction that
> >   extracts a single parameter type:  [...]
>
> Good practice is to typedef the sequence and use mpl::at_c on the new name.
> It also leads to more readable code, IMO.

If you only need to access a single parameter type (for
example because you know the function is unary)

  foo< parmeter_type< F, 0 >::type >();

is much more readable than

  typedef parameter_types< F > params;
  foo< at_c< params, 0 >::type >();

at least in my opinion.

> Further, I think it's a good idea to encourage people to use the MPL.

Agreed.  But I also have to work in the real world, and
unfortunately there are a lot of people who, rightly or
wrongly, are intimidated by the MPL.  By all means encourage
people to use it, but it shouldn't be at the cost of making
the simplest of operations more complicated.

> > 11.  Member function arities
> >
> >   The documentation to function_arity states that the 'this'
> >   pointer is included -- which is what most people would
> >   expect.  It's also what the implementation does.  However
> >   in the 'Use Cases' page of documentation, a R (C::*)()
> >   function is listed in the first code fragment under the
> >   comment '0 parameters'.  Although not explicitly
> >   contradictory (parameters and arity don't *necessarily*
> >   mean the same thing), I think this is potentially
> >   confusing.
>
> I'm afraid that making the same point while addressing
> this issue could be more confusing. Any suggestions?

Remove both the '// 0 parameters' and '// 1 parameters'
comments, leaving the '// ...' and subsequent comment at the
end?

> > 12.  Use cases documentation
> >
> >   To be honest, I find this page of the documentation
> >   dreadful, which is shame as the rest is quite reasonable.
> >   It discusses various scenarios that could benefit from the
> >   application of this library, but utterly fails to suggest
> >   *how* they would benefit from the library.
> >
> >   On the whole page, only one of the library's components is
> >   actually mentioned -- the function_pointer metafunction
> >   for synthesising function types.  But the code fragment is
> >   far too incomplete to be of any real use.  It's also one
> >   of the more complicated examples as it requires using the
> >   MPL.  By all means include such a example, but not as the
> >   only one!
> >
>
> Did you follow the links? There's a lot of inline documentation in the files.
> But maybe I don't get your point.

Yeah, I followed the links, and the examples are very good.
But what I think what's missing are any short code fragments
using the Boost.FunctionTypes library that illustrate the
basic point.

In your 'Introduction', you've got a few good, brief code
fragments; they provide a flavour of how the library is
used, are aimed at about the right level.

The 'Use cases' page starts well -- the accept_function
example illustrates the basic point of the library.  But the
next sentence is where, IMO, the problem starts:

| The combination with a tuples library that provides an
| invoker component, such as Boost.Fusion, allows to build
| callback facilities that are entirely free of repetitive
| code as shown by the interpreter example

... and indeed it does.  It's a good example.  But as the
first example, as a reader of the documentation, I'm hoping
to see something a little more skeletal, first, to
illustrate the use case.

> > 13.  Why is bft::components useful?
> >
> >   For that matter, what *exactly* does it do?
>
> It gives a one-type respresentation of all properties.

How does it distinguish between member functions and
non-member functions?  Or doesn't it?  Given that this
affects the invokation syntax, it seems an important
property.

> And IIRC there is at least one example that shows it in
> action.

I can't see any uses of it in the example/ directory.

> >   First the documentation says
> >
> >   | components<T,ClassTransform>
> >   |   MPL - Front / Back Extensible Random Access Sequence
> >   |   of all component types and property tag
> >
> >   then it says
> >
> >   | Extracts all properties of a callable builtin type, that
> >   | is the result type, followed by the parameter types
> >   | (including the type of this for member function
> >   | pointers).
> >
> >   The former is ambiguous -- it doesn't say what it means by
> >   'component types'; and the latter contradicts it by not
> >   mentioning the property tag.

At the very least, this contradiction should be fixed --
either the MPL sequence does or it doesn't contain the tag.

> >   It also seems not to mention what it actually means by
> >   *the* property tag.  A function can be variadic or
> >   non_variadic; it be default_cc; it may have cv-qualifiers,
> >   which in the case of a const volatile function needs to
> >   tags to express it.  Are these concatenated using bft::tag
> >   or appended one-by-one to the end of the MPL sequence?
>
> The expression models two concepts.

Sorry, which expression models which two concepts?

Or more concretely, what is the MPL sequence for

  components< double __cdecl (C::*)(float, int, ...) const volatile >

is it

  double,
  float,
  int,
  tag< cdecl_cc, variadic, const_qualified, volatile_qualified >

If so, is the order of the parameters to the tag<> template
specified?  And is there a mechansim for querying the parts
of it?

Cheers
Richard Smith