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

From: Richard Smith (richard_at_[hidden])
Date: 2006-11-09 11:49:47

I've spent some time this afternoon reviewing the contents
of the function type library as its functionality is
something I feel is currently missing from Boost and would
like to see added.  Although most of my comments are
negative, I like this library, and would like to see it in
Boost.  I would prefer to see some modifications made first,
but even in the absense of these, I'd still like to see it
added.

There were several different links posted; I'm assuming that
<http://tinyurl.com/qaf5f> is the correct one.

Throughout these comments, I am assuming the namespace alias

  namespace bft = boost::function_types;

1.  Namespace

  The public interface to this library is in the
  boost::function_types namespace.  As a propsective user of
  the library, it feels very much like an extension to the
  type_traits library, and that leaves me wondering why the
  library isn't in the main boost namespace with the other
  type_traits functionality.

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.

  The majority of the time I can't imagine myself making
  much use of any of these tags.  Are they really worth
  while?  The 'Rationale' section of the documentation
  addresses this and considers some alternatives:

  | The first one is just using more templates so every
  | property has to be asked for explicitly. This approach
  | results in more complicated client code if more than one
  | propery has to be checked and in a exponentially larger
  | library interface.
  |
  | The second alternative is having the client pass in bit
  | patterns via non-type template parameters. The logic has
  | to be performed by the client and there are much more
  | error conditions. Further, class templates with non-type
  | template parameters do not work within MPL lambda
  | expressions and can cause problems with older compilers.

  What about the third possibility: leave it up to the
  client to sythesise these more complicated examples.  It
  will involve adding a few more templates: notably
  is_variadic and is_default_cc.  Then, instead of writing

    bft::is_function< T, bft::variadic >::value

  you would write

    boost:is_function<T>::value && boost::is_variadic<T>::value

  ... which is not significantly more verbose.  I think this
  would be my prefered interface as it would avoid the
  duplication of functionality between the type_traits
  library (and the potential for confusion that this would
  cause).

3.  Extensibility

  The internal bitmask implementation underlying the
  property tags does not appear to allow extension by a
  user.  For example, there is a default_cc tag; suppose I
  want to add support for querying the some other calling
  convention.  Can I write a fastcall_cc tag?  Scanning
  through the implementation it would appear not --
  certainly there's no documentation on how to do so.

  If a tag-like syntax is considered desireable, wouldn't
  something like

    template < typename T,
               template <typename> class UnaryTypeTrait >
    struct is_function
      : integral_constant< bool,
          is_function<T>::value && UnaryTypeTrait<T>::value
        >
    {};

  be more extensible?  Or does this have to work with
  compilers that can't handle template template parameters?

4.  Naming

  It seems unlikely that the default_cc tag will be
  particularly heavily used.  Given this, why not give it a
  more meaningful name: default_calling_convention perhaps?

5.  Extern "C" linkage

  What should this print?

    extern "C" { typedef int fn(); }

    int main() {
      std::cout << bfs::is_function<fn, bfs::default_cc>::value
                << std::endl;
    }

  As function types differing only by language linkage are
  distinct types (7.4/1) (even though many compilers don't
  respect this), they can, in principle, have differing
  calling conventions, in which case the extern "C" calling
  convention is not default.

  This may be a non-issue as I don't know whether any of the
  compilers that Boost targets actually take advantage of
  this, and even if they do, I don't even know whether it is
  possible to detect "C" language linkage with template
  metaprogramming (mostly because you can't put extern "C"
  within a template or atemplate within an extern "C"
  block).

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.

  I can't find any definition of 'builtin' in either the
  Standard or TR1, though it is occasionally used in the
  Standard to refer to integral or arithmetic types --
  clearly this isn't what's meant here, though!

  It would appear that in this context 'callable builtin' is
  supposed to include function types, pointers or references
  to function types and pointers to member functions.  It
  does not include classes with an overloaded operator()
  (unsurpisingly -- it would be a very strange definition of
  'builtin' that included them); nor does it include
  pointers to member data, even though they are 'callable'
  per TR1 and are seemingly as 'builtin' as pointers to
  member functions.

  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.

  I also think 'builtin' needs documenting even if only
  superficially; to me 'builtin' is a synonym to
  'fundamental', though there are no callable fundamental
  types.

7.  What is a 'callable scalar'?

  The term 'scalar' is defined in 3.9/10:

  | Arithmentic types, enumeration types, pointer types, and
  | pointer to member types, and cv-qualified versions of
  | these types are collectively called scalar types.

  Note that references, whether to functions or otherwise,
  are *not* scalars.  The is_callable_scalar metafunction
  returns true for references to functions.  This is wrong.

  Also, why is is_callable_scalar useful?  I'm not sure I
  can think of a use case; and if I could, I imagine it
  would be rare enough that it wouldn't be much of an
  inconvenience to sythesise it from is_function and
  is_callable_builtin.

8.  'Tag Types' documentation

  In several places, the 'Tag Types' documentation
  incorrectly gives types two leading underscores.

9.  Extending result_type

  Perhaps this is beyond the intended scope of the library,
  but I think it would be very useful to extend the domain
  of this metafunction to include class types with a
  result_type member type.  The rationale of this is that in
  many ways bft::result_type is a simplified version of
  boost::result_of -- simplified so that the types of the
  invokation arguments need not be known.

  In other words, if both of these compile

    result_of< Fn( T1, T2, ... ) >::type

    result_type< Fn >::type

  compile, they will refer to the same type.   (This is
  also true of the current implementation without my
  suggested extention.)

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:

    template < typename F, std::size_t N,
               class ClassTransform = add_reference<_> >
    struct function_parameter {
      typedef typedef mpl::at<
        function_parameters<F, ClassTransform>, N
      >::type type;
    };

  (I'd suggest base-zero numbering as given here, though
  this might cause confusion in conjunction with the
  first_argument_type, second_argument_type typedefs in
  std::binary_function.)

  Oh, and I'd also like to suggest a quick comment in the
  documentation saying what the '_' is that is given as a
  template argument to add_reference -- a quick

    using mpl::placeholders::_;

  in the synopsis would probably be sufficient.

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.

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!

13.  Why is bft::components useful?

  For that matter, what *exactly* does it do?  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.

  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?

  But more importantly, why do we need this metafunction?
  It does nothing that separate invokations of result_type,
  parameter_types and the various is_* metafunctions can't
  achieve.  And I would *rather* see separate invokations of
  the these -- it would make for much more readble code.

  Oh, and the name isn't very descriptive.

14.  Type sythesis

  ... And now I see what bft::components is supposed to
  achieve.

  But I still don't find the idea of merging the result
  type, parameter types and arbitrary other information
  together into a single type list at all intuitive.

  What's wrong with:

    template < typename Result, class ParameterSeq,
               class PropertiesTag = null_tag >
    struct function_type;

  Much more readable, in my opinion.

  Having said that, I think these metafunctions are an
  important piece of functionality.  It's merely their
  interface I'm concerned about.

I hope these points have been useful -- and sorry for such a
long post if they haven't!

Cheers
Richard Smith