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