[review][mp11] Formal review of Mp11
What follows is my review of the Mp11 library: [Should the library be accepted into Boost?] Yes. I vote for an unconditional ACCEPT. [What is your evaluation of the design?] Excellent. The design where any template can serve as a list, and any template can serve as a meta-function is both elegant and powerful. The additional quoted meta-function support with utilities such as mp_bind, mp_bind_front, and mp_bind_back, addresses the other request I had when reviewing an earlier version of Mp11. While HOMF were never essential, as the article pointed out, users appreciate more ways to one-line things without having to write a helper type. Some additional primitives are desired, as mentioned later in this review (e.g. min and max functionality) but overall the toolkit is already vast and useful. [What is your evaluation of the implementation? Excellent. The focus on performance is definitely appreciated while we still have to depend on libraries for many of these meta-programming primitives. Ideally many of these would be provided in the standard library and have implementations that only a compiler vendor could provide with built in support, and offer performance beyond what a library could offer. In the meanwhile, the performance offered by Mp11 is great, more than sufficient for my needs, and in my opinion also for the majority of users. In comparison, the tiny C++11 meta-programming library that I have implemented, uses only the obvious C++11 implementations for most functionality, and does not offer the same fast compile times as the equivalents in Mp11. The techniques used by Mp11 via C++11 constexpr, C++14 constexpr, and even C++17 fold expressions, besides improving Mp11 performance, were a valuable source to learn from. Suggestion: Can all the workarounds for MSVC versions 1800 and older for the empty list cases be avoided by instead using a helper trait? i.e. template<template<class...> class L, class... T, template<class...> class P> struct mp_find_if_impl<L<T...>, P> { using type = typename mp_find_if_impl_2<P, T...>::type; }; i.e. Introduce mp_find_if_impl_2 which would then have the obvious implementation as before, only without requiring the MSVC workaround? This is only worth considering if you find it to offer some benefit such as: Additional maintainability of the Mp11 code, with less workarounds, and no costs, e.g. no increase in compilation times as a consequence. [What is your evaluation of the documentation? Very good. The reference documentation is clear, and easily understood. More example and tutorial content before the reference section would always be welcome, but not necessary. Suggestion: The two-part article that Peter wrote (which is referenced in the documentation) is in my opinion so valuable for users to gain better understanding and appreciation of the library that it should be included in the library distribution as part of the documentation. Ideally in the same Asciidoc form as the current documentation, as a supplemental document (or Appendix entry). Suggestion: Does it make sense for the documentation to have a section describing the conventions the library uses, for potential contributors as much as users? e.g. If someone was designing an mp_min_element, should it yield the element type result, or the list index size constant result? Is there an established convention of when an error is acceptable for an empty list versus returning an index? Correction: Under heading mp_find_if<L, P>: "mp_find_f<L, P> is an" should be "mp_find_if<L, P> is an". [What is your evaluation of the potential usefulness of the library? Incredibly useful in its current form, and this is only going to increase as the library is extended based on user requests for additional functionality. Almost every meta-programming primitive that I have needed recently either already exists in Mp11, or I am able to implement using Mp11 with a little effort. During this review I replaced the use of my own meta-programming library in a project of mine without any issues. As part of this review I also tried several exercises where I implemented certain traits and meta-functions using Mp11. Of all the exercises that I attempted, the only one where the library lacked the necessary utility was an "mp_min_element" function: This was an implementation of the typical "type_with_alignment" trait, which can easily be implemented using a "min_element" functionality. Mp11 does not have mp_min, mp_max, mp_min_element, mp_max_element presently, and I believe they should be added because they are useful and users will probably find themselves implementing it. Sure, mp_sort exists, but I would not use a sort when a min_element can suffice. In this case, type_with_alignment<N> could use mp_min_element with a predicate that finds the type in an mp_list of several scalar types, that matches has an alignment of at least N and has the smallest size, and then check that the result element type of the mp_min_element is at least N. The litmus test for me for a meta-programming library is the question: Would I use Mp11 in library code? The answer for me is "yes". If it was part of Boost, I would use it in other Boost libraries (that already require C++11 or higher). [Did you try to use the library? With which compiler(s)? Did you have any problems?] Yes. I used the library as a replacement for an existing meta-programming library of mine in real (non-toy, non-exercise) project code that is compiled with g++ 4.8.5 in c++11 mode for x86 and x86_64 Linux, using Boost 1.64.0, on 64-bit RHEL 6 and measured compilation times. I also used the library in a few exercises that I compiled with g++ 7.1 and clang 4.0.0, for x86 and x86_64 Linux, using the current Boost master branch, for c++11 and c++14, on 64-bit Fedora 26, and again measured compilation times. I also compiled and ran the Mp11 unit tests with the compilers, C++ standard modes, architectures, Boost distributions, and operating systems mentioned. [How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?] More than ten hours over this weekend reading the documentation, reviewing the implementation and tests, using Mp11 in a project replacing my own C++11 meta-programming library, using it in various exercises, and running the Mp11 unit tests with the various compilers, C++ standard modes, targets, and Boost distributions listed above. Prior to the preparation for the review, I had spent some time with an earlier version of Mp11 and comparing it to Metal for implementing a few meta-functions, including the "instantiates_" example from my library: instantiates_<T, templates_<std::set, std::multiset, std::unordered_set> > Where instantiates_ should be implemented in terms of "is_instantiation_of_": is_instantiation_of_<T, std::set> Which has the obvious behavior indicated by the trait name. The Mp11 approach provided by Peter for this case, and the relative ease with which I was able to implement the other meta-functions using Mp11 were a convincing demonstration of the usefulness of the library. [Are you knowledgeable about the problem domain?] Like many of us, I had written a meta-programming library as C++11 language features have become available. Before that, if Boost.MPL solved a problem, more often than not it was convenient and useful enough to just use it, even if writing my own implementation would result in faster compilation times. Peter's article, however, opened my eyes to a more elegant design, and changed how I implemented my C++11 meta-programming library, and in general shaped what I think about meta-programming in modern C++. Thank you Peter for your work on Mp11, for submitting it for inclusion in Boost, and for the articles that motivated it and taught the rest of us. Glen
I would like to add my opinion about the mp_ prefix. Many do not like it, but I actually believe it is very nice to have a name that stands out. Apart for reducing the likelihood that there will be a name-clash if the library should get accepted into the standard, there is also the advantage that you can easily see that this is a mp_ function. Other libraries have adopted a standard of simply appending an underscore after the name so that the library char becomes char_. If your eyesight is not perfect, this convention becomes a pain. I have myself spent hours trying to decrypt weird error-messages (most notably in spirit) to realise that an underscore was missing. So hooray for mp_. ;-) /Peter On Sun, Jul 23, 2017 at 7:44 PM, Glen Fernandes via Boost <boost@lists.boost.org> wrote:
What follows is my review of the Mp11 library:
[Should the library be accepted into Boost?]
Yes. I vote for an unconditional ACCEPT.
[What is your evaluation of the design?]
Excellent. The design where any template can serve as a list, and any template can serve as a meta-function is both elegant and powerful. The additional quoted meta-function support with utilities such as mp_bind, mp_bind_front, and mp_bind_back, addresses the other request I had when reviewing an earlier version of Mp11. While HOMF were never essential, as the article pointed out, users appreciate more ways to one-line things without having to write a helper type. Some additional primitives are desired, as mentioned later in this review (e.g. min and max functionality) but overall the toolkit is already vast and useful.
[What is your evaluation of the implementation?
Excellent. The focus on performance is definitely appreciated while we still have to depend on libraries for many of these meta-programming primitives. Ideally many of these would be provided in the standard library and have implementations that only a compiler vendor could provide with built in support, and offer performance beyond what a library could offer. In the meanwhile, the performance offered by Mp11 is great, more than sufficient for my needs, and in my opinion also for the majority of users. In comparison, the tiny C++11 meta-programming library that I have implemented, uses only the obvious C++11 implementations for most functionality, and does not offer the same fast compile times as the equivalents in Mp11. The techniques used by Mp11 via C++11 constexpr, C++14 constexpr, and even C++17 fold expressions, besides improving Mp11 performance, were a valuable source to learn from.
Suggestion: Can all the workarounds for MSVC versions 1800 and older for the empty list cases be avoided by instead using a helper trait? i.e.
template<template<class...> class L, class... T, template<class...> class P> struct mp_find_if_impl<L<T...>, P> { using type = typename mp_find_if_impl_2<P, T...>::type; };
i.e. Introduce mp_find_if_impl_2 which would then have the obvious implementation as before, only without requiring the MSVC workaround? This is only worth considering if you find it to offer some benefit such as: Additional maintainability of the Mp11 code, with less workarounds, and no costs, e.g. no increase in compilation times as a consequence.
[What is your evaluation of the documentation?
Very good. The reference documentation is clear, and easily understood. More example and tutorial content before the reference section would always be welcome, but not necessary.
Suggestion: The two-part article that Peter wrote (which is referenced in the documentation) is in my opinion so valuable for users to gain better understanding and appreciation of the library that it should be included in the library distribution as part of the documentation. Ideally in the same Asciidoc form as the current documentation, as a supplemental document (or Appendix entry).
Suggestion: Does it make sense for the documentation to have a section describing the conventions the library uses, for potential contributors as much as users? e.g. If someone was designing an mp_min_element, should it yield the element type result, or the list index size constant result? Is there an established convention of when an error is acceptable for an empty list versus returning an index?
Correction: Under heading mp_find_if<L, P>: "mp_find_f<L, P> is an" should be "mp_find_if<L, P> is an".
[What is your evaluation of the potential usefulness of the library?
Incredibly useful in its current form, and this is only going to increase as the library is extended based on user requests for additional functionality. Almost every meta-programming primitive that I have needed recently either already exists in Mp11, or I am able to implement using Mp11 with a little effort. During this review I replaced the use of my own meta-programming library in a project of mine without any issues. As part of this review I also tried several exercises where I implemented certain traits and meta-functions using Mp11. Of all the exercises that I attempted, the only one where the library lacked the necessary utility was an "mp_min_element" function:
This was an implementation of the typical "type_with_alignment" trait, which can easily be implemented using a "min_element" functionality. Mp11 does not have mp_min, mp_max, mp_min_element, mp_max_element presently, and I believe they should be added because they are useful and users will probably find themselves implementing it. Sure, mp_sort exists, but I would not use a sort when a min_element can suffice. In this case, type_with_alignment<N> could use mp_min_element with a predicate that finds the type in an mp_list of several scalar types, that matches has an alignment of at least N and has the smallest size, and then check that the result element type of the mp_min_element is at least N.
The litmus test for me for a meta-programming library is the question: Would I use Mp11 in library code? The answer for me is "yes". If it was part of Boost, I would use it in other Boost libraries (that already require C++11 or higher).
[Did you try to use the library? With which compiler(s)? Did you have any problems?]
Yes. I used the library as a replacement for an existing meta-programming library of mine in real (non-toy, non-exercise) project code that is compiled with g++ 4.8.5 in c++11 mode for x86 and x86_64 Linux, using Boost 1.64.0, on 64-bit RHEL 6 and measured compilation times. I also used the library in a few exercises that I compiled with g++ 7.1 and clang 4.0.0, for x86 and x86_64 Linux, using the current Boost master branch, for c++11 and c++14, on 64-bit Fedora 26, and again measured compilation times. I also compiled and ran the Mp11 unit tests with the compilers, C++ standard modes, architectures, Boost distributions, and operating systems mentioned.
[How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?]
More than ten hours over this weekend reading the documentation, reviewing the implementation and tests, using Mp11 in a project replacing my own C++11 meta-programming library, using it in various exercises, and running the Mp11 unit tests with the various compilers, C++ standard modes, targets, and Boost distributions listed above. Prior to the preparation for the review, I had spent some time with an earlier version of Mp11 and comparing it to Metal for implementing a few meta-functions, including the "instantiates_" example from my library:
instantiates_<T, templates_<std::set, std::multiset, std::unordered_set> >
Where instantiates_ should be implemented in terms of "is_instantiation_of_":
is_instantiation_of_<T, std::set>
Which has the obvious behavior indicated by the trait name. The Mp11 approach provided by Peter for this case, and the relative ease with which I was able to implement the other meta-functions using Mp11 were a convincing demonstration of the usefulness of the library.
[Are you knowledgeable about the problem domain?]
Like many of us, I had written a meta-programming library as C++11 language features have become available. Before that, if Boost.MPL solved a problem, more often than not it was convenient and useful enough to just use it, even if writing my own implementation would result in faster compilation times. Peter's article, however, opened my eyes to a more elegant design, and changed how I implemented my C++11 meta-programming library, and in general shaped what I think about meta-programming in modern C++.
Thank you Peter for your work on Mp11, for submitting it for inclusion in Boost, and for the articles that motivated it and taught the rest of us.
Glen
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Sun, Jul 23, 2017 at 11:08 AM, Peter Koch Larsen via Boost <boost@lists.boost.org> wrote:
I would like to add my opinion about the mp_ prefix. Many do not like it, but I actually believe it is very nice to have a name that stands out.
I like it too but let me propose an alternative sure to displease everyone: In addition to mp11::mp_*, the library can provide a set of alternate aliases in the namespace mp11::alternatives::. For example mp11::alternatives::transform, or mp11::alternatives::if_ People who feel strongly about names similar to MPL or who despise the mp_ prefix can simply lift this alternate set of names into the namespace of their choosing. For example: namespace mp = mp11::alternatives; Lets you use mp::transform, mp::if_ Thanks
On 7/23/2017 2:15 PM, Vinnie Falco via Boost wrote:
On Sun, Jul 23, 2017 at 11:08 AM, Peter Koch Larsen via Boost <boost@lists.boost.org> wrote:
I would like to add my opinion about the mp_ prefix. Many do not like it, but I actually believe it is very nice to have a name that stands out.
I like it too but let me propose an alternative sure to displease everyone:
It displeases me and I usually like options. I think such a complication is unnecessary.
In addition to mp11::mp_*, the library can provide a set of alternate aliases in the namespace mp11::alternatives::. For example mp11::alternatives::transform, or mp11::alternatives::if_
People who feel strongly about names similar to MPL or who despise the mp_ prefix can simply lift this alternate set of names into the namespace of their choosing. For example:
namespace mp = mp11::alternatives;
Lets you use mp::transform, mp::if_
Thanks
AMDG On 07/23/2017 12:15 PM, Vinnie Falco via Boost wrote:
On Sun, Jul 23, 2017 at 11:08 AM, Peter Koch Larsen via Boost <boost@lists.boost.org> wrote:
I would like to add my opinion about the mp_ prefix. Many do not like it, but I actually believe it is very nice to have a name that stands out.
I like it too but let me propose an alternative sure to displease everyone:
In addition to mp11::mp_*, the library can provide a set of alternate aliases in the namespace mp11::alternatives::. For example mp11::alternatives::transform, or mp11::alternatives::if_
I'm not fond of the mp_ prefix, but providing multiple names for the same thing for no good reason other than some people's personal preferences is worse.
People who feel strongly about names similar to MPL or who despise the mp_ prefix can simply lift this alternate set of names into the namespace of their choosing. For example:
namespace mp = mp11::alternatives;
Lets you use mp::transform, mp::if_
In Christ, Steven Watanabe
On 24/07/2017 06:15, Vinnie Falco wrote:
I like it too but let me propose an alternative sure to displease everyone:
In addition to mp11::mp_*, the library can provide a set of alternate aliases in the namespace mp11::alternatives::. For example mp11::alternatives::transform, or mp11::alternatives::if_
People who feel strongly about names similar to MPL or who despise the mp_ prefix can simply lift this alternate set of names into the namespace of their choosing. For example:
namespace mp = mp11::alternatives;
Lets you use mp::transform, mp::if_
Rather than try to guess at someone's preferences for alternate naming, you can just let them write their own header file that imports using whatever naming they like. (This used to be a bit of a pain in the butt for templates pre-C++11, but now it's a snap.)
participants (6)
-
Edward Diener -
Gavin Lambert -
Glen Fernandes -
Peter Koch Larsen -
Steven Watanabe -
Vinnie Falco