[Mesa-dev] [PATCH 22/23] glsl/glcpp: Add a catch-all rule for unexpected characters.

[Kristian Høgsberg]

On Fri, Jun 27, 2014 at 9:11 AM, Kristian Høgsberg <krh at bitplanet.net> wrote:
> On Thu, Jun 26, 2014 at 3:19 PM, Carl Worth <cworth at cworth.org> wrote:
>> In some of the recent glcpp bug-fixing, we found that glcpp was emitting
>> unrecognized characters from the input source file to stdout, and dropping
>> them from the source passed onto the compiler proper.
>>
>> This was obviously confusing, and totally undesired.
>>
>> The bogus behavior comes from an implicit default rule in flex, which is
>> that any unmatched character is implicitly matched and printed to stdout.
>>
>> To avoid this implicit matching and printing, here we add an explicit
>> catch-all rule. If this rule ever matches it prints an internal compiler
>> error. The correct response for anyt such error is fixing glcpp to handle
>> the unexpected character in the correct way.
>> ---
>>  src/glsl/glcpp/glcpp-lex.l | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
>> index 3d32cf4..721d8ae 100644
>> --- a/src/glsl/glcpp/glcpp-lex.l
>> +++ b/src/glsl/glcpp/glcpp-lex.l
>> @@ -161,6 +161,10 @@ glcpp_lex_update_state_per_token (glcpp_parser_t *parser, int token)
>>  %option stack
>>  %option never-interactive
>>
>> +       /* Note: When adding any start conditions to this list, you must also
>> +        * update the "Internal compiler error" catch-all rule near the end of
>> +        * this file. */
>> +
>
> We could use %option nodefault instead:
>
>      causes the _default rule_ (that unmatched scanner input is echoed
>      to `stdout)' to be suppressed.  If the scanner encounters input
>      that does not match any of its rules, it aborts with an error.
>      This option is useful for finding holes in a scanner's rule set.
>
> which would be a little more robust against editing the set o start conditions.

Oh, I should have read the rest of the page - there's %option warn to
go with nodefault:

     warn about certain things. In particular, if the default rule can
     be matched but no default rule has been given, the flex will warn
     you.  We recommend using this option always.

which looks like flex can detect this condition at compile time and
warn about it.

Kristian

>>  %x DONE COMMENT HASH UNREACHABLE DEFINE NEWLINE_CATCHUP
>>
>>  SPACE          [[:space:]]
>> @@ -534,6 +538,15 @@ HEXADECIMAL_INTEGER        0[xX][0-9a-fA-F]+[uU]?
>>                 RETURN_TOKEN (NEWLINE);
>>  }
>>
>> +       /* This is a catch-all to avoid the annoying default flex action which
>> +        * matches any character and prints it. If any input ever matches this
>> +        * rule, then we have made a mistake above and need to fix one or more
>> +        * of the preceding patterns to match that input. */
>> +
>> +<INITIAL,DONE,COMMENT,DEFINE,HASH,NEWLINE_CATCHUP>. {
>> +       glcpp_error(yylloc, yyextra, "Internal compiler error: Unexpected character: %s", yytext);
>> +}
>> +
>>         /* We don't actually use the UNREACHABLE start condition. We
>>         only have this action here so that we can pretend to call some
>>         generated functions, (to avoid "defined but not used"
>> --
>> 2.0.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev