
This is my review of Mp11. As I see it, an important use niche for this library is to allow for phasing out of Boost.MPL in C++11 compilers, so my review is heavily biased by this perspective. To get acquainted with the lib, I made the little experiment of porting Boost.MultiIndex from Boost.MPL to Mp11 , results can be seen at: https://github.com/joaquintides/multi_index_mp11 https://github.com/joaquintides/multi_index_mp11/commit/91f2b32cf5a27b3aeaaf... The process was relatively straightforward, 9x% of it was boilerplate substitution of the form s/[typename] mpl::xx<...>::type/mp11::mp_xx<...>. As for the rest, no serious problem was found. Compile times dropped slightly, maybe around 5-10%, but I did't measure rigurously. 1 DESIGN Mp11 makes the very sensible choice of coding metafunctions/type lists as (variadic) class/alias templates. From this principle everything else follows quite naturally. I also commend dispensing with Boost.MPL iterators and using indexes instead. 1.1 I really don't like the mp_ prefix. I understand this is meant to minimize clashes when using namespace::mp11, but the same purpose can be served by simply using namespace mp=boost::mp11. Having to add this pesky mp_ thing always got in the way of seamless porting, for no real benefit. To add more confusion, some functions (those in integer_sequence.hpp and tuple.hpp) don't have the prefix. 1.2 Why are quoted metafunctions codified with a fn member rather than Boost.MPL-honored apply? Why call it quoted metafunctions rather than metafunction classes, as Boost.MPL does? 1.3 I find that these metafunctions are missing/misplaced: pop_back at/at_c (should be in list.hpp) insert/insert_c/erase/erase_c (should be in list.hpp) replace_at/replace_at_c clear (should be in list.hpp) 1.4 Tuple operations are named differently from their C++14/17 counterparts to "avoid ambiguities when both are visible or in unqualified calls". Yet, this policy is not followed in integer_sequence.hpp. I'd rather go the latter way. 1.5 Treatment of sets and maps is very dissimilar. Why mp_map_find but no mp_set_find? Why mp_set_push_(back|front) but no mp_map_push_(back|front)? Why mp_map_erase but no mp_set_erase? I think both interfaces should have the same functions, except possibly mp_map_(replace|update). 1.5.1 I think that, for consistency, mp_map_find should return an index (like mp_find) rather than the element or void. 1.6 Boost.MPL sequences simulate variadic behavior by having boost::mpl::na-defaulted type parameters. As a consequence, the following is somehow unexpected: std::cout<<mp_size<boost::mpl::vector<int,char>>::value<<"\n"; //prints 20 Porting from / coexisting with Boost.MPL would be greatly aided by some utility function to convert Boost.MPL sequences to Mp11 lists: template<typename MplSequence> struct mp_mpl_sequence_impl; template<template<typename...> class M,typename... T> struct mp_mpl_sequence_impl<M<T...>> { using type=mp_remove<mp_list<T...>,boost::mpl::na>; }; template<typename MplSequence> using mp_mpl_sequence=typename mp_mpl_sequence_impl<MplSequence>::type; ... std::cout<<mp_size< mp_mpl_sequence<boost::mpl::vector<int,char>>>::value<<"\n"; // prints 2 1.7 mp_eval[_xx] functions should accept metafunctions in both their true and false branches. As it stands now, we'd have to write stuff like using l1=mp_if_c< b, mp_list<mp_quote<F>,type1,type2>, mp_list<mp_quote<G>,type3,type4>>; using l2=mp_apply_q<mp_first<l1>,mp_rest<l1>>; to emulate (the as of yet non-existent) using l2=mp_eval_if_c<b,F,type1,type2,G,type3,type4>; The current behavior would be served by the new interface with little overhead: (current) mp_eval_if_c<b,T,F,U...> (new) mp_eval_if_c<b,mp_identity_t,T,F,U...> I have to say I don't know how to apply this new behavior to mp_eval_if_q; thinking out loud, we'd need something like mp_eval_if_q<B,Q1,U1...,mp_else,Q2,U2...> which doesn't look too pretty. A more exotic alternative could be mp_eval_if_q<B,Q1(U1...),Q2(U2...)> 1.7.1 Why there's no eval_if_q_c (or eval_if_c_q?) 2 IMPLEMENTATION I had just a cursory look at the code, but everything seems clean and reasonably straightforward. I like the fact that C++14/17 features are taken advantage of when available (e.g. variadic fold expressions). There are many lines wider than 80 chars, but I think this is OK with current Boost guidelines. Testing seems pretty exhaustive. I find it surprising that there's so much boilerplate code repetition in the testing code --I'd have expected Mp11 itself to be used to generate extensive test cases automatically. 3 DOCUMENTATION This is IMHO the weakest part of this submission. I have concerns with both the tutorial and the reference. 3.1 TUTORIAL With template metaprogramming, one has to ask whether a tutorial for a metaprogamming lib should be oriented towards explaining *metaprogramming* or rather the particular design of the library. Mp11 documentation seems to try both ways, and does not excel in either of them. 3.1.1 Examples are heavy handed and do not focus on why Mp11 is useful --too much discussion goes into the particular difficulties of the use cases studied. I consider myself a reasonably advanced metaprogrammer and found it difficult (and, ultimately, useless) to follow the exposition. For a newbie, the examples are just impenetrable; for a seasoned metaprogrammer wanting to learn Mp11, she's better off reading the definitions section only and then jumping into the reference. I suggest taking a look at Brigand tutorials, which are excellent at explaining the lib to entry-level metaprogrammers in a step-by-step fashion. I think the key is Brigand tutorials don't try to tackle industry-grade metaprogramming problems: they just accompany the reader along toy use cases, and that's really enough. 3.1.2 The definitions section, by contrast, is too terse and at points vague to the verge of incorrection: - It is not "template class" but "class template". - It is not made clear whether a list is a class/alias template (say, mp_list) or rather an instantiation of this template (mp_list<int,char>). Same goes for metafunctions. The confusion extends to the reference, where class templates and their instantiations are named the same, for instance: template<class L> using mp_reverse = /*...*/; mp_reverse<L<T1, T2, …, Tn>> is L<Tn, …, T2, T1>. - The above is not a legal subtlety: as it stands, the definition for set doesn't make sense, as it implies that a set is a *class template* that somehow enforces unicity of template type parameters. Same goes for maps. - All in all, seems like the author's intention is to define lists (and sets and maps) as *template class instantiations*: if this is the case then it has to be explained how two different lists (say, the result of pushing back a type to a list) are connected through their common class template. - By contrast, seems like the intention is to define a metafunction as a class/alias template rather than its instantiations (e.g. mp_size is a metafunction but mp_size<my_list> is not). If this is the case, then the assertion that "Another distinguishing feature of this approach is that lists (L<T…>) have the same form as metafunctions (F<T…>) and can therefore be used as such." is false. - There's a latent concept that goes undocumented, namely lists resulting from the instantiation of a non-variadic class/alias template (e.g. std::pair). Some Mp11 metafunctions are not applicable to them (e.g. mp_push_back), but this is not clearly documented. To be clear, I'm not advocating mathematical rigor, but the current fuzziness is confusing at best and can impede understanding at worst. 3.2 REFERENCE In general, the reference is too terse and overlooks important details. 3.2.1 A "Requires" clause should be added to many entries. For example, it is not clear what happens with mp_at_c<L, I> when I is out of bounds (it does not compile). Another example: mp_pop_front<std::pair<int,bool>> does not compile because std::pair<int,bool> is not a "variadic sequence" (missing concept here). 3.2.2 A complexity section should be added, where complexity is number of of template instantiations. For instance, I don't know how costly set operations are. 3.3 PERFORMANCE As already suggested in another review, links to metaben.ch could be added. 4 WHY ANOTHER METAPROGRAMMING LIBRARY? The landscape of C++ metaprogramming libs is quite populated. These libs can be classified into (to follow metaben.ch's terminology) heterogeneous (Boost.Fusion, Boost.Hana) and pure-type (Boost.MPL, Mp11, Brigand, Metal, Kvasir). To be fair, we're dealing here with acceptance of a library into Boost, so we should only be really concerned about intra-Boost clashes, which leaves us with Boost.Fusion, Boost.Hana, Boost.MPL and Mp11. In my opinion, heterogeneous and pure-type libs have different scopes and entry barriers: for the kind of intra-lib scaffolding I find myself doing, I'd rather go with a pure-type metaprogramming lib before buying the complexities of Boost.Hana. In this respect, a replacement for old and clunky Boost.MPL is most welcome, and this is the place that Mp11 can occupy. This said, we can also look out to the wider world and recognize that Mp11 and Brigand have the very same design philosophy and come up with solutions that are exactly interchangeable. It would be great if we could find a way that efforts behind Mp11 and Brigand could be coordinated to come up with something bigger that could serve as Boost's pure-type metaprogramming library as well as a non-Boost lib. To be honest, I don't have a clue how this could be done, or whether there's willingness to collaborate among Mp11's and Brigand's authors. 5 ANSWERS TO REVIEW QUESTIONS * Should Mp11 be accepted into Boost? Please state all conditions for acceptance explicity. My vote is for CONDITIONAL ACCEPTANCE dependent upon proper addressing of, at least, mp_list<_1_1, _1_2, _1_4, _1_5, _1_7, _3_1_2, _3_2_1> Of course I'd like all points of my review to be discussed, but I feel only those listed above are important enough to hold acceptance. * 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? All addressed above, hopefully. * Did you try to use the library? With what compiler? Did you have any problems? Yes. VS 2015. No problems. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? 5-6 hours for the porting exercise, 4-5 hours for the review. * Are you knowledgeable about the problem domain? I've been metaprogramming for 14 years. Thanks to Peter Dimov for his submission of Mp11 and for being such an active contributor to Boost during so many years. Joaquín M López Muñoz