[Review] Type Traits Introspection library by Edward Diener starts tomorrow Friday 1
According to the schedule, review for the Type Traits Introspection library by Edward Diener starts this friday and run till July 10th. =========== What is it? =========== The TTI library, which is an abbreviation for the 'Type Traits Introspection' library, allows a programmer to introspect at compile time the inner elements of a C++ type. The introspection process depends on specifying the name of the inner element by different macros for different types of elements, and then using a generated metafunction to determine whether that element exists within the enclosing type. The inner elements which can be introspected are type, class template, member data, member function, static member data, and static member function. The TTI library is based on the type_traits_ext portion of the Concept Traits Library, with improvements and additions, and also reproduces functionality ( without changing existing code ), for the purposes of completeness, from Boost.MPL regarding introspection of types and templates. The purpose of the library is to provide a consistent set of interfaces for doing compile-time introspection of a type, which other template metaprogrammers can use in their code. If you are at all interested in compile-time introspection of types, please take a look at the functionality of this library. =================== Getting the library =================== The library is available at http://svn.boost.org/svn/boost/sandbox/tti/ Note that this repository will be updated up to next thursday by Edward. =================== Writing a Review =================== The reviews and all comments should be submitted to the developers list, and the email should have "[TTI] Review" at the beginning of the subject line to make sure it's not missed. Please explicitly state in your review whether the library should be accepted. The general review checklist: - What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain? And finally, every review should answer this question: - Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion.
Please explicitly state in your review whether the library should be accepted.
YES, this library should be accepted after some issues are solved.
- What is your evaluation of the design?
The names of include files are not easy to remember because they are not fully conformant to the macro name (sometimes yes, sometimes not): - HAS_TYPE is in type.hpp - HAS_TEMPLATE is in template.hpp - HAS_TEMPLATE_CHECK_PARAMS is in template_params.hpp -> template_check_params.hpp? - HAS_MEMBER_DATA is in mem_data.hpp -> member_data.hpp? - HAS_MEMBER_FUNCTION is in mem_fun.hpp -> member_function.hpp? I would use the lower case version of the macro with BOOST_TTI_HAS_ removed so that it is easy to remember. I agree with others that traits should be generated in the local namespace instead of boost::tti. Then the user has full control of name clash.
- What is your evaluation of the implementation?
I have not been in detail in the implementation. Just opening some files and this is what came out: can you remove trailing spaces?
- What is your evaluation of the documentation?
- simple examples should appear very early in the doc. You could maybe add a tutorial section starting with the simplest examples, leaving the more complex to the end. You should also give an application example of the trait itself. for example (untested): template <class T, bool Has_a=has_member_data<a, int>::value> void print(const T&); template <class T> void print<T, true>(const T &t) { std::cout<<t.a<<'\n'; } template <class T> void print<T, false>(const T&) { } - Table 1.2. . TTI Macro Metafunctions, it is not clear what means "Type with check". . You could add < T > or < T, U > after the trait name (for example boost::tti::has_type_'name' < T >). . BOOST_TTI_HAS_TEMPLATE, BOOST_TTI_VM_HAS_TEMPLATE_CHECK_PARAMS and BOOST_TTI_HAS_TEMPLATE_CHECK_PARAMS: not clear from the table what it checks for. Is there a class inside T that is declared templaste < typename... > name? What if the enclosing type is itself a template but not the inner type. In that case, the inner type is still a template. - the documentation of for each macro is too succinct. You should re-explain the naming convention that will be used by the macro to create the new trait. You could even just give the C++ code that will be produced by the macro (just the visible code): BOOST_TTI_HAS_TYPE(name) is equivalent to: template < typename T, typename U=notype > struct has_type_name { const bool value; }; with value ... In particular, examples are required for each of them. - examples in tti_usingNTM.html In general, I think simple examples miss. Complex examples are given but simple not. For example, for has_type, I quote: "Does T have a nested type called 'DType' within 'BType::CType' ? BOOST_TTI_HAS_TYPE(DType) boost::tti::mf_has_type < boost::tti::has_type_DType<_>, CTypeNM
We could just have easily used the boost::tti::mf_valid_member_type metafunction to the same effect: boost::tti::mf_valid_member_type < DTypeNM
"
I would be happy to see simple examples like the following: BOOST_TTI_HAS_TYPE(B) BOOST_TTI_HAS_TYPE(mytrait, B) struct A { struct B { } }; boost::tti::has_type_B<A>::value; // true boost::tti::has_type_B<int>::value; // false boost::tti::has_type_B<B>::value; // false boost::tti::mytrait<A>::value; // true boost::tti::mytrait<int>::value; // false boost::tti::mytrait<B>::value; // false - typos: . in tti_detail.html: "will tell use" -> "will tell us" or "will tell the user" . in tti_nested_type.html: "The goal of the TTI library is never to produce a compiler" -> "The goal of the TTI library is to never produce a compiler" - I have seen no explanation of the implementation behavior with regards to public/protected/private category of inner type/member. Could give details in the documentation? I imagine there is no need to introspect private/protected types/members. But what happens if the type/member exists but is private? Compilation error or value==false returned? By the way, I have seen that this is currently not tested as all data and types are public in the tests (in structs). I have tested the following program: #include <iostream> #include <boost/tti/type.hpp> #include <boost/tti/mem_data.hpp> class A { private: class private_type { }; int private_data; protected: class protected_type { }; int protected_data; public: class public_type { }; int public_data; }; BOOST_TTI_HAS_TYPE(private_type) BOOST_TTI_HAS_TYPE(protected_type) BOOST_TTI_HAS_TYPE(public_type) BOOST_TTI_HAS_MEMBER_DATA(private_data) BOOST_TTI_HAS_MEMBER_DATA(protected_data) BOOST_TTI_HAS_MEMBER_DATA(public_data) int main() { std::cout<<boost::tti::has_type_private_type<A>::value<<'\n'; // true std::cout<<boost::tti::has_type_protected_type<A>::value<<'\n'; // true std::cout<<boost::tti::has_type_public_type<A>::value<<'\n'; // true std::cout<<boost::tti::has_member_data_private_data<A, int>::value<<'\n'; // compile time error std::cout<<boost::tti::has_member_data_protected_data<A, int>::value<<'\n'; // compile time error std::cout<<boost::tti::has_member_data_public_data<A, int>::value<<'\n'; // true return 0; } has_type answers true for private, protected and public member types but has_member_data fails to compile if the data member is private of protected (tested with g++ 4.4.4 (log attached). I do not know if this is possible or not but I think the behavior should be: - private or protected -> value==false even if the type/member exists - public -> value==true because private and protected are implementation details which should be ignored. For example what is the need to know if member m of type double exists? probably to use m somewhere. But if m is private, the fact that m exists does not help very much.
- What is your evaluation of the potential usefulness of the library?
I am sure that it can be usefull to implement the best algorithms to appropriate types. For example, you can imagine that adding an element at the front of a container could be specialized for containers that have member function push_front by using it and for other containers by using more complex copying algorithm. Then the best algorithm is used tranparently to the final user.
- Did you try to use the library? With what compiler? Did you have any problems?
Yes, I tried to use the library with g++ 4.4.4. It worked as expected apart for private members as reported (tested HAS_TYPE and HAS_MEMBER_DATA). I ran tests with intel 11 and 12 which failed but the author is dealing with this. I ran tests with g++ 4.4.4 and nothing failed (tested tti target only).
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I read the doc + some testing (about 4 hours).
- Are you knowledgeable about the problem domain?
I am a bit aware of metaprogramming as I am trying to add an extension to the type trait library for detecting existence of operators.
And finally, every review should answer this question:
- Do you think the library should be accepted as a Boost library?
YES. Frédéric
On 7/15/2011 1:20 PM, Frédéric Bron wrote:
Please explicitly state in your review whether the library should be accepted.
YES, this library should be accepted after some issues are solved.
Appreciated !
- What is your evaluation of the design?
The names of include files are not easy to remember because they are not fully conformant to the macro name (sometimes yes, sometimes not): - HAS_TYPE is in type.hpp - HAS_TEMPLATE is in template.hpp - HAS_TEMPLATE_CHECK_PARAMS is in template_params.hpp -> template_check_params.hpp? - HAS_MEMBER_DATA is in mem_data.hpp -> member_data.hpp? - HAS_MEMBER_FUNCTION is in mem_fun.hpp -> member_function.hpp?
I would use the lower case version of the macro with BOOST_TTI_HAS_ removed so that it is easy to remember.
I agree with this also and will change the names of the header files accordingly.
I agree with others that traits should be generated in the local namespace instead of boost::tti. Then the user has full control of name clash.
I will be removing the generated macro metafunctions from the boost::tti namespace and not putting them in any namespace. I have been duly chastised for my short-sightedness <g>.
- What is your evaluation of the implementation?
I have not been in detail in the implementation. Just opening some files and this is what came out: can you remove trailing spaces?
- What is your evaluation of the documentation?
- simple examples should appear very early in the doc. You could maybe add a tutorial section starting with the simplest examples, leaving the more complex to the end.
My intention for the documentation is to change it so that each macro metafunction gets its own topic with a fuller explanation and a set of examples starting with the simplest to the more complex.
You should also give an application example of the trait itself.
for example (untested): template<class T, bool Has_a=has_member_data<a, int>::value> void print(const T&); template<class T> void print<T, true>(const T&t) { std::cout<<t.a<<'\n'; } template<class T> void print<T, false>(const T&) { }
I can understand the need for practical application examples in the documentation. The problem, as I see it, is that whatever application examples I give others will find much better practical uses. Perhaps it would be better just adding an examples subdirectory below libs and creating some examples which are actual small applications, with enough documentation in the source files of these examples so that the end-user could get an idea of how TTI may be used. For instance I notice that type traits has some basic example programs and I can create some for TTI and then just inline the code in the documentation the way type traits does it.
- Table 1.2. . TTI Macro Metafunctions, it is not clear what means "Type with check". . You could add< T> or< T, U> after the trait name (for example boost::tti::has_type_'name'< T>).
The dodcumentation does say: class T = enclosing type class U = type to check against and then clicking on the metafunction names takes you to the reference for that metafunction. But as I said above I will give a more extensive explanation with examples for each metfunction, with some examples.
. BOOST_TTI_HAS_TEMPLATE, BOOST_TTI_VM_HAS_TEMPLATE_CHECK_PARAMS and BOOST_TTI_HAS_TEMPLATE_CHECK_PARAMS: not clear from the table what it checks for. Is there a class inside T that is declared templaste< typename...> name?
Click on the metafunction name for more info. BOOST_TTI_HAS_TEMPLATE just checks for a template 'name' inside T with all class/typename template parameters ( from 1 to 5 template parameters currently ). BOOST_TTI_VM_HAS_TEMPLATE_CHECK_PARAMS and BOOST_TTI_HAS_TEMPLATE_CHECK_PARAMS check for an exact match of a template's parameters of a specific 'name' inside T. The first uses variadic macro syntax to give the exact template parameters while the second uses a pp-seq to give the exact template parameters. All three produce a metafunction which takes an enclosing type and returns a true or false value. In an extensive discussion with others I have decide to subsequently change these macro metafunctions to a single BOOST_TTI_HAS_TEMPLATE, which will work similarly but are syntactically different depending on whether variadic macros are supported.
What if the enclosing type is itself a template but not the inner type. In that case, the inner type is still a template.
All enclosing 'types' are types. Templates themselves are not introspected. But a type's inner template may be found through introspection.
- the documentation of for each macro is too succinct. You should re-explain the naming convention that will be used by the macro to create the new trait. You could even just give the C++ code that will be produced by the macro (just the visible code):
BOOST_TTI_HAS_TYPE(name) is equivalent to: template< typename T, typename U=notype> struct has_type_name { const bool value; }; with value ...
In particular, examples are required for each of them.
I hear you. I really don't want to duplicate the source code for each in the doc. Some are pretty long and involved.
- examples in tti_usingNTM.html
In general, I think simple examples miss. Complex examples are given but simple not. For example, for has_type, I quote: "Does T have a nested type called 'DType' within 'BType::CType' ? BOOST_TTI_HAS_TYPE(DType) boost::tti::mf_has_type < boost::tti::has_type_DType<_>, CTypeNM
We could just have easily used the boost::tti::mf_valid_member_type metafunction to the same effect: boost::tti::mf_valid_member_type < DTypeNM
"
I will give simpler examples first, then more complex. As mentioned above, each macro metafunction will get its own topic with individual examples which explains the metafunction usage more clearly.
I would be happy to see simple examples like the following: BOOST_TTI_HAS_TYPE(B) BOOST_TTI_HAS_TYPE(mytrait, B)
struct A { struct B { } };
boost::tti::has_type_B<A>::value; // true boost::tti::has_type_B<int>::value; // false boost::tti::has_type_B<B>::value; // false
boost::tti::mytrait<A>::value; // true boost::tti::mytrait<int>::value; // false boost::tti::mytrait<B>::value; // false
Great idea ! I will definitely do that and follow that format for the simpler examples.
- typos: . in tti_detail.html: "will tell use" -> "will tell us" or "will tell the user"
Corrected !
. in tti_nested_type.html: "The goal of the TTI library is never to produce a compiler" -> "The goal of the TTI library is to never produce a compiler"
I do not think that either variation is more correct than the other, but this is one area of English grammar about which I admit I am unsure.
- I have seen no explanation of the implementation behavior with regards to public/protected/private category of inner type/member. Could give details in the documentation?
I actually forgot about this completely when I created tests. I definitely fell asleep in this area. Ideally one should be able to introspect only public inner elements successfully, since one is introspecting from outside the class, while protected and private element introspection should return false. Another reviewer brought this up and I realized that TTI might benefit from the ability to introspect protected inner elements and private inner elements. I have thought about ways to do this, aided by other code I have seen, and I have some plans, but I have delayed trying them out until the review was over and I could work on the changes suggested by others.
I imagine there is no need to introspect private/protected types/members. But what happens if the type/member exists but is private? Compilation error or value==false returned?
Ideally it should never be a compiler error, but just return 'false'. In reality I may not be able to guarantee that. So further below for a brief discussion in my response.
By the way, I have seen that this is currently not tested as all data and types are public in the tests (in structs).
This is where I nodded <g>. Furthermore my tests did not really test const/volatile cases but I think this is much less of an eventual problem. I really must expand my tests.
I have tested the following program: #include<iostream>
#include<boost/tti/type.hpp> #include<boost/tti/mem_data.hpp>
class A { private: class private_type { }; int private_data; protected: class protected_type { }; int protected_data; public: class public_type { }; int public_data; };
BOOST_TTI_HAS_TYPE(private_type) BOOST_TTI_HAS_TYPE(protected_type) BOOST_TTI_HAS_TYPE(public_type) BOOST_TTI_HAS_MEMBER_DATA(private_data) BOOST_TTI_HAS_MEMBER_DATA(protected_data) BOOST_TTI_HAS_MEMBER_DATA(public_data)
int main() { std::cout<<boost::tti::has_type_private_type<A>::value<<'\n'; // true std::cout<<boost::tti::has_type_protected_type<A>::value<<'\n'; // true std::cout<<boost::tti::has_type_public_type<A>::value<<'\n'; // true std::cout<<boost::tti::has_member_data_private_data<A, int>::value<<'\n'; // compile time error std::cout<<boost::tti::has_member_data_protected_data<A, int>::value<<'\n'; // compile time error std::cout<<boost::tti::has_member_data_public_data<A, int>::value<<'\n'; // true return 0; }
has_type answers true for private, protected and public member types but has_member_data fails to compile if the data member is private of protected (tested with g++ 4.4.4 (log attached).
I do not know if this is possible or not but I think the behavior should be: - private or protected -> value==false even if the type/member exists - public -> value==true because private and protected are implementation details which should be ignored.
I totally agree with what you feel the results should be. Now to some explanations. I am not sure offhand why the protected/private types are found but I will look at it. The code is really the MPL code for BOOST_MPL_HAS_XXX_TRAIT_DEF/BOOST_MPL_HAS_XXX_TRAIT_NAMED_DEF which I am just reusing in TTI. I will look at that to see if I can tweak it accordingly. I know why the protected/private members give a compiler error and there may not be a way around it, but I will certainly try to find one. The problem is that SFINAE is used to determine if the address of the member can be taken ( the same goes for member functions, static data, and static data members ). According to current SFINAE, but not as I understand it to extended SFINAE in C++0x, the data can have its address taken without it being considered a SFINAE failure even if the data is not accessible. Then of course once that member is chosen as valid, an access error occurs because it is not accessible. As I understand it with extended SFINAE in C++0x, extended SFINAE does kick in to rule out the member function if it is not accessible, therefore giving not a compiler error, but a false condition, which is what we want. However the number of compilers supporting C++0x SFINAE are probably small right now.
For example what is the need to know if member m of type double exists? probably to use m somewhere. But if m is private, the fact that m exists does not help very much.
I agree. I will work on this area to correct it.
- What is your evaluation of the potential usefulness of the library?
I am sure that it can be usefull to implement the best algorithms to appropriate types. For example, you can imagine that adding an element at the front of a container could be specialized for containers that have member function push_front by using it and for other containers by using more complex copying algorithm. Then the best algorithm is used tranparently to the final user.
- Did you try to use the library? With what compiler? Did you have any problems?
Yes, I tried to use the library with g++ 4.4.4. It worked as expected apart for private members as reported (tested HAS_TYPE and HAS_MEMBER_DATA).
I ran tests with intel 11 and 12 which failed but the author is dealing with this.
It is really an MPL problem too with Intel 11/12, which I reported and assigned to Joel Falcou. I will help with it as much as possible. The Sun C++ compiler also has the same problem, along with a few more, which I will be looking at.
I ran tests with g++ 4.4.4 and nothing failed (tested tti target only).
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I read the doc + some testing (about 4 hours).
- Are you knowledgeable about the problem domain?
I am a bit aware of metaprogramming as I am trying to add an extension to the type trait library for detecting existence of operators.
And finally, every review should answer this question:
- Do you think the library should be accepted as a Boost library?
YES.
Frédéric
Thanks very much for the review, the suggestions, and finding the private/protected problems, even if I belatedly realized what most of them are from some earlier remarks about the library by others. Eddie Diener
. BOOST_TTI_HAS_TEMPLATE, BOOST_TTI_VM_HAS_TEMPLATE_CHECK_PARAMS and BOOST_TTI_HAS_TEMPLATE_CHECK_PARAMS: not clear from the table what it checks for. Is there a class inside T that is declared templaste< typename...> name?
Click on the metafunction name for more info.
Yes but it would be better to understand it directly from the table.
- the documentation of for each macro is too succinct. You should re-explain the naming convention that will be used by the macro to create the new trait. You could even just give the C++ code that will be produced by the macro (just the visible code):
BOOST_TTI_HAS_TYPE(name) is equivalent to: template< typename T, typename U=notype> struct has_type_name { const bool value; }; with value ...
In particular, examples are required for each of them.
I hear you. I really don't want to duplicate the source code for each in the doc. Some are pretty long and involved.
In fact just the following would be enough: template< typename T, typename U=notype> struct has_type_name;
My tests did not really test const/volatile cases but I think this is much less of an eventual problem.
OK for const but I would not bet on volatile... Good job Eddie. Cheers, Frédéric
On 7/16/2011 4:05 PM, Frédéric Bron wrote:
. BOOST_TTI_HAS_TEMPLATE, BOOST_TTI_VM_HAS_TEMPLATE_CHECK_PARAMS and BOOST_TTI_HAS_TEMPLATE_CHECK_PARAMS: not clear from the table what it checks for. Is there a class inside T that is declared templaste< typename...> name?
Click on the metafunction name for more info.
Yes but it would be better to understand it directly from the table.
I agree that a short explanation for each metafunction generated should be in the table. For a longer explanation the user can click on the link and go to the reference.
- the documentation of for each macro is too succinct. You should re-explain the naming convention that will be used by the macro to create the new trait. You could even just give the C++ code that will be produced by the macro (just the visible code):
BOOST_TTI_HAS_TYPE(name) is equivalent to: template< typename T, typename U=notype> struct has_type_name { const bool value; }; with value ...
In particular, examples are required for each of them.
I hear you. I really don't want to duplicate the source code for each in the doc. Some are pretty long and involved.
In fact just the following would be enough: template< typename T, typename U=notype> struct has_type_name;
That sounds sensible.
My tests did not really test const/volatile cases but I think this is much less of an eventual problem.
OK for const but I would not bet on volatile...
I understand your trials with operator traits and volatile. My approach may end up being that since 'volatile' is used so rarely, getting it to work correctly in all cases may not be necessary, even if it is ideal.
Good job Eddie. Cheers,
Thanks ! Eddie
participants (3)
-
Edward Diener -
Frédéric Bron -
Joel Falcou