Boost logo

Boost :

Subject: Re: [boost] [review][mp11] Formal review of Mp11
From: Robert Ramey (ramey_at_[hidden])
Date: 2017-07-23 17:34:00


On 7/15/17 3:19 AM, Bjorn Reese via Boost wrote:

Here is my review.

> Please answer the following questions in your review [4]:
>
> 1. Should Mp11 be accepted into Boost? Please state all conditions
> for acceptance explicity.
YES
>
> 2. What is your evaluation of the design?
Incredible
>
> 3. What is your evaluation of the implementation?
fantastic
>
> 4. What is your evaluation of the documentation?
excellent - but see notes below
>
> 5. What is your evaluation of the potential usefulness of the library?
beyond believe
>
> 6. Did you try to use the library? With what compiler? Did you have
> any problems?
no. Actually I was unclear whether I need C++11 or C++14
>
> 7. How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
Not much - Spend maybe 2 hours trying to figure out the documentation.
Not enough time - but I feel I have a fairly good understanding of
>
> 8. Are you knowledgeable about the problem domain?
More than average, but not as much as some

Observations:

Much has been said about the mp_ prefix. I'm not crazy about it as I
would much prefer to use mp11::transform rather than mp11::mp_transform.
  But I'm not really bothered by it all that much. It seems that Peter
has made a case for it and it's being discussed. I don't much about
this so I'm happy with whatever is finally used.

Calling this a library does not do it justice. It's nothing less than
the text book of how C++11+ should/can be used. I would like to see
even more examples and expository examples. It should really be a book
- sequel to Abrahams and Gurtovoy's book which itself was sequel to
Alexandrescu's book.

I started reading the documents and kept these notes. I quit when I ran
out of gas.

==============

Incorporation of Simple C++ meta programming and sequel int to the
documentation directly rather than by reference. I'm guessing this
might be part of a section - "how the library is implemented".

I would also like to see a section which describes the parallels with
boost mpl. I see this is a newer, better, simpler, faster, more modern,
etc. replacement for mpl. Assuming I'm correct I would like to see this
described in a separate section titled something like -
background/history or something similar.

Definitions

A "quoted metafunction" is a class with a public metafunction member
called fn, ...

I presume that this is the same as the mpl notion of metafunction class
which must contain a public metafunction member called "apply"

A "map" is a list of lists, the inner lists having at least one element
(the key.) The keys of the map must be unique.

 From the examples, I can't discern what the key is. Is it the first
element of each set? or ...

Generating test cases

This is a great motivating example. For the safe numerics library I
used the Boost.Preprocessor library for this purpose. Had this existed
at the time, I would have turned to it first. It's a use case which is
useful in the real world.

It's unclear whether one needs C++11 or C++14 in order to use this library.

Writing common_type Specializations

This example is more complicated and I'm having a hard time
understanding it. This is not a criticism - I just think I have invest
more effort. I'm motivated as I believe it has some relevence to some
stuff I'm currently working on.

But I'm having trouble with the example:

I seems that mp_defer is related if not equivalent to the mpl notion of
"eval_if". I'm not clear on this since mp_defer isn't really describe

The example also reference mp_transform whose purpose is not obvious.

One thing that might be making it difficult for me is the usage of
common_type_t for the example metafunction name which uses
std::common_type in it's implementation. Then there is common_tuple.
Perhaps altering the names might make all this more understandable to
the normal brain.

Soooo - consider:

a) thinking some more about the names
b) inserting comments into the example code
c) perhaps reformation the code somewhat keep the comments in a good place
d) including information - a cheat sheet which relates notions in this
library to notions/nomenclature in boost.mpl. Or consider using the MPL
nomenclature. This latter idea is intersting as this is clearly meant
to be the "modern" version of MPL.
e) thie common_type_t is just an alias for std::common_type. I'm not
convinced it clarifies anything.

With all that the example might look more like:

"Let’s write a common_type specialization for two std::tuple arguments
which we'll call common_tuple."

Hmmm - what is this going to return? It sounds like that it's going to
produce a new tuple where the first element is the common_type of first
element of each constituent tuple, the second is the common_type of
second tuple and so forth. But that's just a guess though

create a tuple from

template<class Tp1, class Tp2> using common_tuple =
     // mp_transform does ... what - returns a tuple (li
     mp_transform<
         std::common_type, // from standard C++14
         Tp1, // first tuple ?
         Tp2 // second tuple ?
>;

Then specialize std::common_type to use the above ...

OK have written the above it might be making sense to me now. My basic
point is that these examples need a lot of extra care as they are
essential to explaining what the library is for an how to use it to most
programmers. I've worked many years with MPL and still I have
difficulty understanding all this.

Looking back I see that mp_transform IS described in the overview. I
didn't pay much attention to the overview as reading something labeled
"overview" is for wimps. Now that I look back on it I see that I should
have read it. The first sentence exactly and succinctly describes what
the library does. I think this sentence should have it's own paragraph.

This library is not really a library. Its an unlibrary.

"As another example, consider the hypothetical type expected<T, E…​>"

Another excellent example. Although its related to the previous one, it
should have it's own heading. It's particularly interesting given all
the thunder and lightening that this topic has recently received.

"Fixing tuple cat"

This example would require many hours and referring to another paper to
figure out. I don't think it's suitable as an introduction to the library.

"Computing Return types"

Another good example. From the first paragraph I understood the problem
and the proposed solution using variant. Given that I'm already getting
burned out from reading this stuff, I decided to declare victory and
move on.

"Reference"

I absolutely loved having the table of contents in the left hand window
of the documentation. I fund it very, very, very helpful and intuitive.
  I do question the fact that reference section includes just a couple
of lines for each library function. I much prefer and learned a lot
from the boost mpl documentation which includes a small compilable
example for each function. Maybe you might lift those examples and
include them here. A lot of work I know, but that's why you get paid
the big bucks. Actually I would prefer for web documentation that each
metafunction have it's own page, with type requirements for the
parameters, and small self contained examples.

This is not really a library, it's an exposition how to use of C++11++.
It's a way of training the next generation of programmers to use TMP.
They will be standing on your shoulders by means of this document. I
think you way underestimate this document and library will have on the
future.

===========

Robert Ramey


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk