[Mesa-dev] [PATCH] glsl: Fix glcpp to catch garbage after #if 1 ... #else

[Matt Turner]

On Thu, Jun 12, 2014 at 11:26 AM, Carl Worth <cworth at cworth.org> wrote:
> Previously, a line such as:
>
>         #else garbage
>
> would flag an error if it followed "#if 0", but not if it followed "#if 1".
>
> We fix this by setting a new bit of state (lexing_else) that allows the lexer
> to defer switching to the <SKIP> start state until after the NEWLINE following
> the #else directive.
>
> A new test case is added for:
>
>         #if 1
>         #else garbage
>         #endif
>
> which was untested before, (and did not generate the desired error).
> ---
>  src/glsl/glcpp/glcpp-lex.l                               | 10 +++++++++-
>  src/glsl/glcpp/glcpp-parse.y                             |  6 ++++--
>  src/glsl/glcpp/glcpp.h                                   |  1 +
>  src/glsl/glcpp/tests/103-garbage-after-else-0.c          |  3 +++
>  src/glsl/glcpp/tests/103-garbage-after-else-0.c.expected |  4 ++++
>  src/glsl/glcpp/tests/103-garbage-after-else.c            |  3 ---
>  src/glsl/glcpp/tests/103-garbage-after-else.c.expected   |  4 ----
>  src/glsl/glcpp/tests/123-garbage-after-else-1.c          |  3 +++
>  src/glsl/glcpp/tests/123-garbage-after-else-1.c.expected |  4 ++++
>  9 files changed, 28 insertions(+), 10 deletions(-)
>  create mode 100644 src/glsl/glcpp/tests/103-garbage-after-else-0.c
>  create mode 100644 src/glsl/glcpp/tests/103-garbage-after-else-0.c.expected
>  delete mode 100644 src/glsl/glcpp/tests/103-garbage-after-else.c
>  delete mode 100644 src/glsl/glcpp/tests/103-garbage-after-else.c.expected
>  create mode 100644 src/glsl/glcpp/tests/123-garbage-after-else-1.c
>  create mode 100644 src/glsl/glcpp/tests/123-garbage-after-else-1.c.expected
>
> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
> index 188e454..dea6a6c 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -125,7 +125,7 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
>          * as set by the parser to know whether to change from INITIAL
>          * to SKIP or from SKIP back to INITIAL.
>          *
> -        * Three cases cause us to switch out of the SKIP state and
> +        * Four cases cause us to switch out of the SKIP state and
>          * back to the INITIAL state:
>          *
>          *      1. The top of the skip_stack is of type SKIP_NO_SKIP
> @@ -142,9 +142,17 @@ HEXADECIMAL_INTEGER        0[xX][0-9a-fA-F]+[uU]?
>          *         "#elif". Even inside an "#if 0" we need to lex this
>          *         expression so the parser can correctly update the
>          *         skip_stack state.
> +        *
> +        *      4. The lexing_else bit is set. This indicates that we
> +        *         are lexing tokens after #else and before the
> +        *         newline. We don't want to skip here just so that we
> +        *         can correctly flag errors if there is any
> +        *         garbage (other than comments) on the same line as the
> +        *         #else.
>          */
>         if (YY_START == INITIAL || YY_START == SKIP) {
>                 if (parser->lexing_if ||
> +                    parser->lexing_else ||

The preprocessor is one of the only bits that uses tabs. I'd rather we
stick with that, or convert the whole thing.

Otherwise, looks good to me.

Reviewed-by: Matt Turner <mattst88 at gmail.com>