Re: [boost] [Review] Type Traits Introspection library by Edward Diener starts tomorrow Friday 1

Subject: Re: [boost] [Review] Type Traits Introspection library by Edward Diener starts tomorrow Friday 1
From: John Maddock (boost.regex_at_[hidden])
Date: 2011-07-05 04:38:44

OK here's my review of this library.

First off the headline - yes I believe it's a useful library that should be 
accepted into Boost, subject to some of the comments below.

>     - What is your evaluation of the design?

Probably too many macros - see detailed comments below, otherwise fine.

>     - What is your evaluation of the implementation?

The library is more or less a meta-library around MPL (or else uses MPL-like 
implementation techniques, and IMO that's the right way to go.

>     - What is your evaluation of the documentation?

Nice and complete, but confused me in places (see below).

>     - What is your evaluation of the potential usefulness of the
>       library?

Very useful, I could have used this a few times in the past.

>     - Did you try to use the library?  With what compiler?  Did you
>       have any problems?

There are issues with the Intel compiler which Edward is investigating - see 
also detailed comments below

>     - How much effort did you put into your evaluation? A glance? A
>       quick reading? In-depth study?

Full read of the docs, and about an hour playing with the code.  Will spend 
some more time getting to the bottom of the Intel issue shortly.

>     - Are you knowledgeable about the problem domain?

As I maintain the type traits lib I hope so, though I confess to *not* being 
a hard core meta-programmer :-0

>     - Do you think the library should be accepted as a Boost library?

Yes, subject to the comments below.

Detailed comments:
~~~~~~~~~~~~~

1) I agree with the other comments that it's basically just plain wrong to 
insert the generated traits classes in a namespace: they should appear 
within the current namespace scope.  Under "General Functionality" the 
"Important" block should (IMO) encourage users to place macro invocations 
within a private "detail" namespace to avoid name clashes from multiple use 
in multiple headers.
2) Further to the above, I wonder if it's worthwhile providing a set of 
"standard" introspection classes for the common cases, a straw man list 
would be something like:

Types: type, value_type, size_type, difference_type, iterator, 
const_iterator, pointer, const_pointer, reference, const_reference.
Static Values: value
Functions: begin(), end(), swap(self_type)

3) I was confused by the section "Macro Metafunction Name Generation", I 
think some examples would help a lot.  Does the variety of macros supplied 
get simplified by just dumping the generated trait in the current namespace?

4) BOOST_TTI_MEMBER_TYPE looks like it would be more useful (to me anyway!) 
if the generated template would accept a second optional parameter that is 
the value of the ::type member when the type being checked does not have the 
specified inner type (hope that makes sense).

5) The description of "Table 1.4. TTI Nested Type Macro Metafunction 
Existence" makes no sense to me (though it might later in the docs of 
course).

6) I found code formatting such as:

boost::tti::has_static_member_data_DSMember
  <
  T,
  short
  >

A little hard to read, as long as the lines don't get too long I would much 
prefer:

has_static_member_data_DSMember<T, short>

7) I'm not sure about the term "composite types": to me it's a member 
function pointer type, or if you prefer a function signature.  So instead of 
BOOST_TTI_HAS_COMP_MEMBER_FUNCTION I'd prefer 
BOOST_TTI_HAS_MEMBER_FUNCTION_WITH_SIG.  In fact, I think the version with 
function-signature should be the main variant, so maybe 
BOOST_TTI_HAS_MEMBER_FUNCTION(IntFunction) would become:

has_member_function_IntFunction<int (T::*)(short)>

and some other name should be provided for the mpl::sequence version - if 
it's there at all?

I'm also thinking that what folks really want to know is:  "Is there a 
function named X, that can be called with arguments of types A, B and C". 
It would be interesting to see how far along this road we could actually 
get?

8) I confess I haven't done any metaprogramming lately, but the purpose of 
the sections "Macro Metafunctions as Metadata" and "Nullary Type 
Metafunctions" completely eluded me.  So I'd like to know if this is just me 
;-)

My gut feeling at present is that the macros in these sections don't add 
much to the library and will likely confuse the heck out of new users, so I 
guess I'd like to see better descriptions and some killer use cases before 
accepting these parts.... but more opinions please!

9) Great shame about the nested function template issue - hopefully someone 
can find a workaround.

10) Small point, but in the reference docs, for example for 
BOOST_TTI_TRAIT_HAS_COMP_MEMBER_FUNCTION(trait, name), it says:

"returns = a metafunction called "boost::tti::trait" where 'trait' is the 
macro parameter."

But it doesn't really return anything, rather it's a code generator.  So I'd 
rather it said that it generates a metafunction:

template<class T>
struct trait
{
   static const value = unspecified;
   typedef mpl::bool_<true-or-false> type;
};

Then go on to define T, value, and type (I assume there is a member named 
type??).

11) In the docs the main header file should be given as boost/tti/tti.hpp 
rather than just tti.hpp.

12) Testing BOOST_TTI_HAS_COMP_MEMBER_FUNCTION(begin)

I see that this static assertion fails:

BOOST_STATIC_ASSERT((boost::tti::has_comp_member_function_begin<std::vector<int>::const_iterator 
(std::vector<int>::*const)(void)>::value));

Have I done something silly?  Testing const-member functions seems OK with 
BOOST_TTI_HAS_MEMBER_FUNCTION BTW.

Regards, John.