On Fri, 21 Jul 2017 17:36:11 +0200 Bjorn Reese via Boost <boost@lists.boost.org> wrote:
The formal review of Mp11 continues until (and including) Monday, July 24th. [...snip..] Please answer the following questions in your review [4]:
1. Should Mp11 be accepted into Boost? Please state all conditions for acceptance explicity.
Yes, I think it should be accepted.
2. What is your evaluation of the design?
I like the simplicity and consistency. In particular, I like how it is designed to explicitly work with any template-class parameter pack, so that `variant` and other custom user types can be manipulated directly. Also, moving away from MPLs vector metafunctions that return different types is a huge benefit - its frustrating trying to get any partial template "match" with that design.
3. What is your evaluation of the implementation?
I did not read the entirety of the code (unfortunately), but the parts I did read were what I would expect. The primary negative is that some of the techniques are likely non-obvious to someone new to metaprogramming (the SFINAE techniques in `mp_and_` for instance), but I am not aware of any obvious alternative solutions that requires less C++ hidden knowledge with equivalent performance.
4. What is your evaluation of the documentation?
I think the links to the blog post at the beginning should be made internal to the documentation so that it is included with the Boost release. The documentation is also less rigorous than the documentation for MPL. An example would be: For an Mp11 integral constant type T, T::value is an integral constant in the C++ sense. For example, std::integral_constant<int, 7>, or struct N { static int constexpr value = 2; }; which is the entirety of the discussion about integral constants. Should `decltype(T::value)` only be a built-in type? Does it ever make sense for user types with proper operator overloads? The MPL documentation was overly verbose at times, but I think it could help negotiate expected behavior between users and the library maintainers. For example, when trying to squash some bugs in the C++11 fusion::vector, I realized that all but one C++03 fusion sequence could implicitly-construct from a longer sequence (the remaining elements were ignored). The documentation never mentioned this behavior, but it also never specified that the sequences had to be identical in length. I decided to "bring-back" that behavior in C++11 fusion::vector, because I found a way to do it with no discernable impact. But if it was specified as a non-feature, perhaps the maintenance is easier. Unfortunately I have no specific recommendations other than to perhaps continually update the design section in particular as the library is being updated/maintained.
5. What is your evaluation of the potential usefulness of the library?
The prior discussion about metaprogramming being easier with C++11 was an interesting one. Since it is easier, many bespoke implementations are likely to exist in codebases. But I can see a real advantage to internal use by several Boost libraries. `make_integer_sequence` has been implemented numerous times (fusion and spirit x3 both have one now IIRC), and the easiest implementation is linear-recursive and therefore sub-optimal. In another instance I managed to write a decent `mp_and_` with the inevitable MSVC tantrum discovered shortly later. So if included into Boost, I might try to persuade Joel to get fusion's C++11 code and spirit x3 to use mp11 in a few places where performance could be improved with hopefully no broken client code. I suspect there are _many_ places throughout Boost where this process could occur.
6. Did you try to use the library? With what compiler? Did you have any problems?
I did not try to use the library, I primarily read the implementation instead. Reading the implementation might be more useful than using it in this context. I was also interested in several similar libraries that have been discussed on the mailing list. After reviewing them quickly, I think this is the best solution to this specific problem domain _right now_. I primarily like the explicit goal to treat all template-class variadics identically, grouping similar metafunctions in a single file (the implementations are terse; brigand _seems_ like it would take longer to include/load all of the different files for some of the algorithms something like fusion or spirit would need), more focused/complete documentation, and not tackling a larger/different problem domain (kvasir).
7. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I spent probably 3-4 hours reading the documentation, code, mailing list posts, and took a minimal amount of time to look at several alternative libraries (metal, meta, brigand, kvasir).
8. Are you knowledgeable about the problem domain?
Fairly knowledgeable. I've hacked at the implementations of spirit and fusion a bit, so I know the exact number of times to hit your head against the wall to make that SFINAE expression work. Always 8. Lee