On 05/28/2015 03:12 PM, Christophe Henry wrote:
Please always include in your review a vote as to whether the library should be accepted into Boost.
I vote yes for acceptance into Boost.
- What is your evaluation of the design?
Clean parser combinator design.
- What is your evaluation of the implementation?
Well-structured and easy to read.
- What is your evaluation of the documentation?
Very thorough and of a high standard. What kind of grammar can be written with Metaparse? I fould a reference to EBNF; does this mean that I can write context-free grammars? If so, how is left-recursion [1] handled? I would be nice to have this kind of information in the documentation. The "Getting Started" chapter has much more detail about the internal machinery than I would expect from an introduction. I found the "User Manual" chapter to be better as an introduction. I still think all the content in "Getting Started" is useful, but it may be an idea to restructure these chapters. Furthermore, the tutorial links to an external document. [1] http://en.wikipedia.org/wiki/Left_recursion
- What is your evaluation of the potential usefulness of the library?
The library seems mostly to target embedded DSLs, but I would be more inclined to choose, say, sqlpp11 for embedded SQL than a text-based SQL approach. That said, others may find text-based expressions useful. I still see the ability to transform string literals into types and values as very useful, and something that could find surprising applications. For instance, I would love to see something that could generate a perfect hash function for a set of strings at compile-time. Even a hash parser to turn a string into a hash value could be useful.
- Did you try to use the library? - With what compiler? - Did you have any problems?
I ran the test suite and a couple of examples with gcc 4.8. No problems were encountered.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
A couple of hours spent reading the documentation, and another couple of hours looking through the code. My code reading mainly focussed on the string type, the int_ parser, and the traversal functions.
- Are you knowledgeable about the problem domain?
I have been writing DSL parsers for a living, although they were all run-time parsers. The following are minor comments directed more towards the author than the review manager. Why are the errors in separate headers? Implementations are found in the v1 folder, but their include guards have V2 in them (probably an oversight from the recent move to a separate repository.) The tests are not re-compiled the second time you run them. As this is a compile-time library, I was wondering if a re-compilation should be forced (if that is doable with Boost.Build.)