Re: [boost] [metaparse] Review period starts May 25th and ends June 7th - ongoing

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

Hi Roland,

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 
example.

> 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.

Regards,
   Ábel