[boost] [metaparse] Update review

Subject: [boost] [metaparse] Update review
From: Edward Diener (eldiener_at_[hidden])
Date: 2015-06-04 00:45:30

This is an updated review of the metaparse library. In my initial review 
I reacted to the fact that library presentation had changed during the 
review period. Although I feel I made a point I was too harsh in doing 
so. So now I would like to review the library as it has been presented 
in its form suitable for review as a modular boost library.

- What is your evaluation of the design?

The design of the library appears well thought out to me, providing both 
low-level and higher-level metafunctions for use. The higher level 
metafunctions take other metafunctions as input in a classic design of 
metafunction programming as explained in MPL. The design of the library 
makes it easy for anybody with knowledge of MPL to understand how to use 
the metafunctions in the library and to create one's own higher level 
metafunctions for particular parsing tasks if necessary.

A couple of quibbles about the design based merely on my own sense of 
'naming'. Since I have roundly criticized "naming wars" <g> in the past 
I no doubt leave myself open for criticism, but these are just personal 
suggestions, which I can live without:

* BOOST_STRING is very generic and can easily lead to a macro clash. 
Something like BOOST_METAPARSE_STRING or maybe BOOST_MPS_STRING would be 
more distinct and safer. I favor shortened names for libraries in macros 
to be 3 characters rather than 2 characters so as to decrease macro clashes.

* The various 'foldxxx' names, even with the Cheat-Sheet explanation of 
their name formation in the docs, seems needlessly confusing. Would it 
not be better to make longer, less confusing names, which actually spell 
out what the metafunctions are about. Is it really so hard to write 
"fold_left', 'fold_right', 'fold_left_fail' etc. etc. Sometimes clarity 
trumps how many keys one types.

- What is your evaluation of the implementation?

I didn't look at the code of the implementation. I generally trust that 
if the code works as documented an experienced programmer's 
implementation is his own affair.

- What is your evaluation of the documentation?

The documentation is fairly profuse approaching the subject of 
compile-time parsing in many different ways. I found that the work 
needed to produce a solid set of documentation had been done. I have 
some quibbles and a few suggestions so let me give it as a general list 
of items:

* A  general discussion about the purpose of the functionality of the 
library would be welcome. Such as the possible metaprogramming scenarios 
in which a programmer would want to accept and analyze ( parse ) an MPL 
string in creating his metafunctions.

* A general comparison of when it might be better/worse to use Metaparse 
as oppose to Boost.Spirit. Pluses and minuses of both approaches might 
be discussed.

* On the first page of documentation it mentions the various types of 
output which Metaparse can generate. Both 'types' and 'metafunctions' 
are very understandable, but whatever is meant by "Objects" and 
"Callable C++ functions" doesn't follow the definition of these terms in 
C++ which I know. Some discussion, or link, of what these two types of 
output mean in the context of Metaparse should be given.

* The Versioning section appears to be wrong in quite a number of 
places. Please proofread what was written there and correct/update it.

* In the Reference section, among the items enumerated at the end are 
'Metafunctions and metafunction classes' and "Utilities' but when you 
click on the links to go the actual headings it looks like the 
metafunctions are split before and after the utilities.

- What is your evaluation of the potential usefulness of the library?

I think it give the template programmer another important tool for 
designing templates for end-users. As such it has intrinsic value for 
programmers writing templates for end-users to use.

- Did you try to use the library?

Yes.

   - With what compiler?

I used the Metashell tool to test things out. I then tried manually 
compiling some examples in the documentation with Mingw/gcc-4.9.2 with 
'-std=c++11'. I also ran the tests successfully with Mingw/gcc-4.9.2 
with '-std=c++11'.

   - Did you have any problems?

No. Everything worked as expected.

- How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?

My main effort was in trying to understand how the parts of the library 
worked with each other. It is not easy to understand everything, and I 
do not claim that I do, but I could understand from the documentation 
how the major areas of parsing worked and what metafunctions are used 
for what tasks. I could also see that if would be easy to incorporate 
one's own metafunctions into generalized parsing tasks using what 
Metaparse provides. I did need the full documentation set, which is 
profuse, to understand the parts of the library.

Part of my initial reaction when the library was first presented was 
that I could only find the documentation for the 'Getting Started' link, 
and I am one of those people who is just mentally incapable of 
understanding code from the point of view of "here's how to use" 
documentation only. Once I discovered the full documentation in the 
updated presentation of the library in modular Boost form I could see 
and understand the library much better.

- Are you knowledgeable about the problem domain?

Generally knowledgable about parsers and how they work, but not an expert.

Finally I vote YES to accept the library into Boost, reversing my 
previous initial vote. The library will prove useful for template 
programming in some contexts, has obviously been designed thoughtfully, 
and the full documentation presents everything one would need to use the 
library, explaining things in various effective ways, one or more of 
which ways should satisfy all programmers who do template programming.