Subject: Re: [boost] [metaparse] Review period starts May 25th and ends June 7th - ongoing
From: Abel Sinkovics (abel_at_[hidden])
Date: 2015-06-08 15:26:13
Thank you for the review.
On 2015-06-08 08:02, Roland Bock wrote:
> I believe that there is still room for improvement regarding the error
> messages in case of a parse error. I described the idea of a portable
> static_assert in my talk at MeetingC++ in Berlin last year: For each
> potential problem (i.e. for each of the BOOST_STATIC_ASSERTs that you
> now have), define a struct with a static method that fires a
> static_assert when called. In case of an error, transport this class
> from the problem-location to the TMP-call-site and call that static
> method. This way you can produce a very targeted message with a very
> short TMP call stack.
> Haven't looked deep enough to say if this pattern is actually applicable.
Metaparse uses classes (used as values in the metaprograms) representing
parsing errors. The library provides a few (eg. index_out_of_range,
unpaired, etc) and users can create more (a helper macro is provided).
The built-in ones are in the metaparse::error namespace. Parsing errors
are propagated out of the top level parser and the user of the library
can decide if he wants to handle them himself or uses build_parser,
which emits a compilation error using static assertion. When someone
builds the interface of his library using Metaparse, there might be
template layers above the parser, in which case the author of those has
to propagate the parsing errors out of those layers and then emit the
compilation error at the top level to follow your advice. (he can do
that with Metaparse).
The parsing error classes have a static get_value method giving a string
explanation of the error. Unfortunately, this can only be displayed at
runtime (static assert's explanation has to be a string literal).
What might be done based on your portable static assertion approach is
adding a static method to these error classes representing the
compilation errors to "static assert themselves" with better error
messages. My concern about this is loosing the details of the error
(position in the parsed text, the range boundaries and the index in case
of an out of range error, etc). I'll check if "static assert yourself"
can be done without loosing that information.
> I tried a few of the examples in https://github.com/sabel83/metaparse
> using clang-3.5.1 -std=c++11.
> The only problem I encountered was in example/parsing_error. It did not
> produce an error at first. I had to remove a comment. It turned out
> there was a spelling error in the comment leading to a compile error
> that was not intended, I guess. After fixing that, I think I saw the
> intended error message which I found a bit hard to grok.
I'll fix the typo. The idea there is that the commented code fails to
compile, however commenting that out and using debug_parsing_error can
display the above mentioned (very user-friendly) text explanation of the
error. I'll add some further explanation on this to the README of the
> I was a bit surprised to see BOOST_STATIC_ASSERT in a C++11 environment.
To the best of my knowledge BOOST_STATIC_ASSERT uses static_assert, when
it is available. So using it works (well) everywhere and Metaparse does
not need to deal with that aspect of the platform.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk