On Thu, Feb 29, 2024 at 4:44 PM David Sankel via Boost
# Are you knowledgeable about the problem domain?
Yes. I've written several parser combinator libraries in C++ and used still more in C++ (Spirit v1, v2, and qi) and other programming langauges (Haskell's Parsec, attoparsec, and others).
# How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I read through the tutorial documentation and some of the reference documentation. I also tried to create several examples with the library and made comparisons to Haskell's Parsec.
# What is your evaluation of the potential usefulness of the library?
Very
# What is your evaluation of the documentation?
The documentation isn't great. Consider char_ https://tzlaine.github.io/parser/doc/html/boost/parser/char_.html's documentation. It includes no examples and fails to mention it can be used without arguments.
I can certainly mention that in that reference entry. Ticket: https://github.com/tzlaine/parser/issues/150 FWIW, char_ is used without arguments *all over* the docs. It would be hard to miss it. [snip]
# What is your evaluation of the design?
Much of the wealth of research into parser combinators wasn't incorporated into this library. I didn't see in the documentation, for example, an easy way to "map" one parser's attribute into another. This kind of thing should be a basis operation provided by the library.
Could you explain what you mean by this? This happens implicitly usually, and you can explicitly do it in semantic actions. Do you mean something else? An implicit example would be: struct s {int i, double d; }; std::vector<s> s_vec; bool success = bp::parse("...", *(bp::int_ >> double_), bp::ws, s_vec); The sequence parser on the inside is feeding its attributes into the repeat-parser on the outside.
# What is your evaluation of the implementation?
The usage of has_include is a ODR nightmare for large codebases. It would be better to generate some kind of config.hpp that sets these definitions at compile time.
This is a great point. There are three places where this currently happens: 1) Boost.TypeIndex vs typeinfo (for printing type names in the trace) 2) BOOST_ASSERT vs. assert 3) Spirit X3 vs. charconv vs. Boost.Charconv 1 and 3 are easily addressed by adding an extra template parameter to the few function templates that use those APIs, so a mismatch will be a build break, not ODR. 2 does not apply to production code, and any use of BOOST_ASSERT already has the same problem anyway. Ticket: https://github.com/tzlaine/parser/issues/151
For custom variant/tuple types, user specialization of variable templates is also an ODR nightmare. Again, a config.hpp would be better for this. For this, though, I don't think the configurability is worth the complexity. We have standard types for these so we should use them.
Huh? There's no ODR here. I don't use one type if the specialization exists and another if it does not. The issue is that if you feed me an attribute out-arg, I might not treat it as what it is unless you tell me that it's variant-like, or optional-like, or whatever. For instance, Andrzej was trying to use boost::variant in one of his parsers, and the printing code broke. It broke because boost::variant defines ostream & op<<(ostream&,variant), and then *static asserts in it*. What? Anyway, without the printing code knowing that it's a variant type, it doesn't know to skip printing it (I don't visit variants during printing, I just print <<variant>> or whatever). [snip]
Overall, I think the syntax has too much focus on magically filling in user-provided structures instead of the basics of monadic parser combinators and basis operations.
Sure, this parser lib focuses on attribute generation more than any one other feature, to be sure. That's part of its legacy -- Spirit 2 and Spirit X3 have the same focus, and I'm a fan.
Consider the following Haskell parser which evaluates simple parenthesized sum expressions. I wasn't able to use Boost.Parser to accomplish this after reading the documentation and several attempts. I'm sure it's possible, but I don't see how it can be done using the combiners and primitives provided.
import Text.Parsec.String (Parser)import Text.Parsec integer :: Parser Integerinteger = do n <- many1 digit return (read n) integerPlus :: Parser IntegerintegerPlus = do x <- integer y <- (try $ char '+' >> expression) <|> return 0 return $ x+y parentheses :: Parser Integerparentheses = do char '(' x <- expression char ')' return x expression :: Parser Integerexpression = integerPlus <|> parentheses
No idea what any of that means. Thanks for reviewing! Zach