Re: [boost] [review][mp11] Reminder of ongoing formal review

Subject: Re: [boost] [review][mp11] Reminder of ongoing formal review
From: Lee Clagett (forum_at_[hidden])
Date: 2017-07-25 16:37:09

On Fri, 21 Jul 2017 17:36:11 +0200
Bjorn Reese via Boost <boost_at_[hidden]> 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