Re: Late review of Boost.Parser

From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2024-03-01 01:17:58

On Thu, Feb 29, 2024 at 4:44 PM David Sankel via Boost
<boost_at_[hidden]> wrote:
>
> # 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