Re: [boost] [review][mp11] Formal review of Mp11

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 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" 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