Re: [boost] [TTI] Review for The Type Traits Introspection library by Edward Diener **extended**

Subject: Re: [boost] [TTI] Review for The Type Traits Introspection library by Edward Diener **extended**
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2011-07-17 14:22:02

On Sun, Jul 17, 2011 at 8:10 AM, Edward Diener <eldiener_at_[hidden]>wrote:

> On 7/16/2011 10:58 PM, Jeffrey Lee Hellrung, Jr. wrote:
>
[...]

> First, a general comment.  There's a lot more in this library than I
>> expected at first, some of which is welcome but some of which I think
>> might
>> be unnecessary, in that, e.g., it only provides a slight convenience, or
>> only addresses a (in my opinion) obscure use case.  You (Eddie) may argue
>> that there's no harm in including it, as users can simply ignore it, but I
>> disagree.  I believe there is value in being judicious about what to
>> include
>> and not include.  It simplifies the presentation of the library, making it
>> easier to digest; and it reduces maintenance.
>>
>
I think I'll have to reference these comments below.

> * I would suggest a name change from "TTI" to "Introspection" or something
>> similar.  I have a feeling that, looking at a source file with "#include
>> <boost/tti/header.hpp>" would give one less of an idea of what the purpose
>> of that header is than "#include<boost/introspection/header.hpp>", and
>> likewise for macros, "BOOST_TTI_MACRO" vs. "BOOST_INTROSPECTION_MACRO".  I
>> don't think the length of the name should be a huge concern since I would
>> imagine the ratio of metafunction-generating-macro use to
>> generated-metafunction use is much less than 1, i.e., the macro will not
>> be
>> used frequently relative to the metafunction that's generated.
>>
>
>
> If I changed it from TTI to something longer I will get complaints that the
> macro names are too long ( some are already long ). As far as the suggested
> "Introspection" it does seem vaguer to me than "Type Traits Introspection",
> byn which I meant to suggest introspecting a type.
>

Yeah, I understand on both accounts.  Maybe I'm just being paranoid.

>  * I'm confused what the purpose of the *_GEN_BASE and *_GEN macros are,
>> and
>> suspect they are probably unnecessary.
>>
>
> Once the enclosing namespace is removed, I will remove the *_GEN_BASE set
> and the *_GEN set will become what the current *_GEN_BASE set is. The
> purpose of the macro is to just generate the name of the metafunction
> without the end-user having to understand the naming scheme.
>

Well, if the naming scheme is simple (I think it is), I don't think we have
to worry about the metafunction provider nor the metafunction user not
understanding the naming scheme.  Have users been requesting these GEN
macros?  Are the GEN macros targeted toward the metafunction provider (who
should know what identifiers he's injecting into the namespace) or the
metafunction user (who will need to read documentation either way to get the
name of the metafunction)?  I don't really see the point, and I now refer
you to my comment about "less is more" at the top.

 * I *think* all generated metafunctions should also expose a nested "type"
>> typedef in addition to a nested "value" static bool in order to be fully
>> Boost.MPL compatible, but...I'm not sure on this one.  For some reason,
>> I've
>> always followed this practice, I think because it allows has_type_xxx<T>
>>  to
>> be used as a nullary Boost.MPL metafunction.  Can someone comment on this?
>>
>
> They do supply a nested 'type', whose 'type::value' is the same as the
> 'value'. I have not documented this because the nested 'type' is unimportant
> in using the metafunctions. The metafunction generated by
> BOOST_TTI_MEMBER_TYPE(name) does have just a nested 'type', which is
> important and used.
>

Okay, as I hadn't looked yet at the implementation, I didn't know it had a
nested type.  It *is* worth documenting, I think.

>> * I think it would be more helpful if the examples were paired with the
>> reference section of the corresponding macro.
>>
>
> Do you mean a link to the appropriate reference section for the macro in
> the code for the examples ? Or do you mean you want an example added to
> reference for each macro ?
>

The latter, e.g., as is done in the Boost.MPL documentation.

>  * I don't see a compelling use case for "has type with check", where
>> "check"
>> checks if the type is the same as some given type.  I would think it would
>> be more useful for "check" to evaluate some Boost.MPL metafunction (and
>> the
>> old behavior is arguably clearer now: boost::is_same<  U,
>> boost::mpl::_1>),
>> and then you can roll this functionality into the "no check" TTI
>> metafunction by defaulting the MPL metafunction template parameter to
>> boost::mpl::always<  boost::true_type>  (or similar).  Indeed, I *think*
>> this
>> would remove the need for BOOST_TTI_MEMBER_TYPE for the use case you
>> present
>> (see below).
>>
>
> When a nested 'type' is a typedef, the metaprogrammer may want to check if
> the typedef is some given 'type' That is why the "has type with check"
> exists.
>

I understand; perhaps I should've explicitly asked:  Why are you limiting it
to that particular query?  Surely the metaprogrammer may want to check if
the typedef has other properties as well, no?

 * I like Lorenzo's idea to condense some related macros into a single
>> macro:
>>     - I think BOOST_TTI_HAS_TEMPLATE can subsume
>> BOOST_TTI_HAS_TEMPLATE_CHECK_**PARAMS and
>> BOOST_TTI_VM_HAS_TEMPLATE_**CHECK_PARAMS.
>>
>
> I told Lorenzo that I will be rolling these into a BOOST_TTI_HAS_TEMPLATE
> for all 3 cases, which will be slightly different for variadic and
> non-variadic macro support.
>

Good.

>      - I think BOOST_TTI_HAS_MEMBER_FUNCTION can subsume
>> BOOST_TTI_HAS_COMP_MEMBER_**FUNCTION, since I think you could just
>> dispatch on
>> whether the first template parameter to the generated metafunction is a
>> pointer-to-member-function or not.  Another convenient syntax may be
>> has_member_function_xxx<  T, Return ( Arg0, Arg1 )>, so that all of the
>> following are equivalent:
>>         has_member_function_xxx<  Return (T::*)( Arg0, Arg1 )>
>>         has_member_function_xxx<  T, Return ( Arg0, Arg 1 )>  // can still
>> use lambda expressions for T, has similar format as other introspection
>> metafunctions
>>         has_member_function_xxx<  T, Return, boost::mpl::vector2<  Arg0,
>> Arg1
>>
>>> // ugliest, but most flexible with Boost.MPL
>>>>
>>>     - Likewise, I think BOOST_TTI_HAS_STATIC_MEMBER_**FUNCTION can
>> subsume
>> BOOST_TTI_HAS_COMP_STATIC_**MEMBER_FUNCTION, and likewise I think the
>> syntax
>> has_static_member_function_**xxx<  T, Return ( Arg0, Arg1 )>  would be
>> convenient.
>>     - Although this doesn't have to do with condensing macros, it would
>> make
>> the library a little more consistent to allow pointer-to-member syntax for
>> BOOST_TTI_HAS_MEMBER_DATA, e.g., has_member_data_xxx<  U T::*>.
>>     - Lastly, BOOST_TTI_HAS_STATIC_MEMBER_**FUNCTION( xxx ) can be
>> reexpressed
>> as BOOST_TTI_HAS_MEMBER_FUNCTION( static xxx ), and likewise for
>> BOOST_TTI_HAS_STATIC_MEMBER_**DATA.
>>
>
> Figuring out the different template parameters and their number at
> compile-time may be possible. I will look into it.
>

Looks like it would be a simple matter of dispatching on
is_member_function_pointer, is_function, and is_member_object_pointer from
Boost.TypeTraits.  Oops, I just found an inconsistency between your
terminology (member data) and Boost.TypeTraits (member object)...

>  * For BOOST_TTI_HAS_TEMPLATE where you supply the template signature, I'm
>> actually fine with the old system of wrapping the signature in parentheses
>> and replacing all commas with ")(" to make a Boost.PP Seq, and prefer it
>> to
>> the array syntax you've proposed since.  That said, I would probably,
>> ultimately, prefer Lorenzo's syntax (e.g., "TTI_HAS_TEMPLATE( template(
>> class, class ) struct tmpl )"), but I guess we'll agree to disagree on
>> what's easier to read (readability should win over writability, right?).
>> For the syntax taking advantage of variadic macros that you've proposed
>> during the review (e.g., "TTI_HAS_TEMPLATE( tmpl, template<  class, class>
>> )"), I worry about usability issues when you're just stringing the
>> template
>> signature into the __VA_ARGS__ argument.  I don't use variadic macros yet,
>> so my sense of "good practice" is very malleable at the moment, but, that
>> being said, it seems like good practice is to use variadic macro arguments
>> only when (a) your macro logically takes a fixed number of arguments, but
>> where you can syntactically leave off trailing arguments that you wish to
>> be
>> defaulted; or (b) your macro takes a varying number of arguments, but the
>> semantic difference between each argument is purely and strictly
>> positional.  To me, the template signature is a completely separate
>> argument
>> from the template name, and the syntax should reflect that, e.g.,
>> TTI_HAS_TEMPLATE( tmpl, (template<  class, class>) ) or TTI_HAS_TEMPLATE(
>> tmpl, ( class, class ) ).  It also seems like this would make it easier to
>> use TTI_HAS_TEMPLATE within other preprocessor metaprogramming constructs
>> (e.g., if I wanted to generate an entire family of metafunctions via
>> BOOST_PP_REPEAT or BOOST_PP_ITERATE), but perhaps you can argue otherwise.
>>
>
> I will have to disagree with your belief that variadic macro syntax should
> not be used.
>

:: grumble ::

>  * When introspecting for nested templates and member functions, how
>> "precise" is the result?  I.e., if you're checking if T has member
>> function
>> xxx with signature void ( ), and T in fact has a member function xxx with
>> signature void ( int = 0 ), is the result false?  (I'm guessing yes, and
>> this should be noted in the reference.)  Similarly for nested templates
>> with
>> default template parameters.
>>
>
> For functions and data the precision should match that of the compiler
> without considering default value(s). 'void ( int = 0 )' still has a basic
> signature of 'void (int) and not 'void ()' so the result should be false.
> But you have alerted me to provide tests to check out function signatures
> with default values.
>

I presume templates with default values is the same deal.  I also presume,
from your preceding comment, that checking compatible template signatures is
not supported.  E.g., I want to check if T::template tmpl<0> is
syntactically well-formed, which it would be if T had a nested template
declared as template< int, class = void > struct tmpl or template< unsigned
int > struct tmpl, but I suspect that TTI does not provide the facilities to
do this.

 * Please document in which scopes you may invoke the
>> metafunction-generating
>> macros.
>>
>
> I will do that but let me just reiterate that the metafunction-generating
> macros generate metafunctions, and therefore anywhere a metafunction can be
> used is where the macros can be used.
>

You mean, "anywhere a metafunction can be *defined* is where the macros can
be used", correct?  I wasn't sure if you were, e.g., creating dummy
namespaces with the macro, which would limit the metafunction-generating
macros to namespace scope.

 * Similar to above, I'm not convinced of the utility of the
>> metafunction-class-generated macros...aren't they largely obviated by
>> boost::mpl::quote[n]?
>>
>
> I created them for convenience, because I found them easy to use as an
> alternative. I do understand now that quote can be used instead, as well of
> course as placeholder expressions.
>

'quote'ing them should address any compile-time efficiency concerns.

If there is a strong feeling from others that I should remove the MTFC
> macros I will do so.
>

Let me again refer you to my "less is more" comment at the top.

- Jeff