[Mesa-dev] [PATCH v2] glsl: Fix glcpp to catch garbage after #if 1 ... #else
Anuj Phogat
anuj.phogat at gmail.com
Thu Jun 12 17:19:46 PDT 2014
On Thu, Jun 12, 2014 at 12:58 PM, 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).
> ---
>
> Matt Turner <mattst88 at gmail.com> writes:
>> The preprocessor is one of the only bits that uses tabs. I'd rather we
>> stick with that, or convert the whole thing.
>
> Thanks for the catch. There are a handful of lines indented with spaces that
> have managed to sneak in, but I might as well not add more. So I've fixed
> that. Also, here in v2, instead of adding a lexing_else bit next to lexing_if,
> instead I'm reusing the original bit and renaming it to lexing_directive.
>
> src/glsl/glcpp/glcpp-lex.l | 25 +++++++++++-----------
> src/glsl/glcpp/glcpp-parse.y | 6 +++---
> src/glsl/glcpp/glcpp.h | 2 +-
> src/glsl/glcpp/tests/103-garbage-after-else-0.c | 3 +++
> .../tests/103-garbage-after-else-0.c.expected | 4 ++++
> src/glsl/glcpp/tests/103-garbage-after-else.c | 3 ---
> .../glcpp/tests/103-garbage-after-else.c.expected | 4 ----
> src/glsl/glcpp/tests/123-garbage-after-else-1.c | 3 +++
> .../tests/123-garbage-after-else-1.c.expected | 4 ++++
> 9 files changed, 31 insertions(+), 23 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..d5fb087 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -137,14 +137,15 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
> * 2. The skip_stack is NULL meaning that we've reached
> * the last #endif.
> *
> - * 3. The lexing_if bit is set. This indicates that we
> - * are lexing the expression following an "#if" of
> - * "#elif". Even inside an "#if 0" we need to lex this
> - * expression so the parser can correctly update the
> - * skip_stack state.
> + * 3. The lexing_directive bit is set. This indicates that we are
> + * lexing a pre-processor directive, (such as #if, #elif, or
> + * #else). For the #if and #elif directives we always need to
> + * parse the conditions, (even if otherwise within an #if
> + * 0). And for #else, we want to be able to generate an error
> + * if any garbage follows #else.
> */
> if (YY_START == INITIAL || YY_START == SKIP) {
> - if (parser->lexing_if ||
> + if (parser->lexing_directive ||
> parser->skip_stack == NULL ||
> parser->skip_stack->type == SKIP_NO_SKIP)
> {
> @@ -193,25 +194,25 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
>
> <SKIP,INITIAL>{
> {HASH}ifdef {
> - yyextra->lexing_if = 1;
> + yyextra->lexing_directive = 1;
> yyextra->space_tokens = 0;
> return HASH_IFDEF;
> }
>
> {HASH}ifndef {
> - yyextra->lexing_if = 1;
> + yyextra->lexing_directive = 1;
> yyextra->space_tokens = 0;
> return HASH_IFNDEF;
> }
>
> {HASH}if/[^_a-zA-Z0-9] {
> - yyextra->lexing_if = 1;
> + yyextra->lexing_directive = 1;
> yyextra->space_tokens = 0;
> return HASH_IF;
> }
>
> {HASH}elif/[^_a-zA-Z0-9] {
> - yyextra->lexing_if = 1;
> + yyextra->lexing_directive = 1;
> yyextra->space_tokens = 0;
> return HASH_ELIF;
> }
> @@ -348,7 +349,7 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
> if (parser->commented_newlines) {
> BEGIN NEWLINE_CATCHUP;
> }
> - yyextra->lexing_if = 0;
> + yyextra->lexing_directive = 0;
> yylineno++;
> yycolumn = 0;
> return NEWLINE;
> @@ -357,7 +358,7 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
> /* Handle missing newline at EOF. */
> <INITIAL><<EOF>> {
> BEGIN DONE; /* Don't keep matching this rule forever. */
> - yyextra->lexing_if = 0;
> + yyextra->lexing_directive = 0;
> return NEWLINE;
> }
>
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 650d0d4..6906e62 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -364,7 +364,7 @@ control_line:
> glcpp_warning(& @1, parser, "ignoring illegal #elif without expression");
> }
> }
> -| HASH_ELSE {
> +| HASH_ELSE { parser->lexing_directive = 1; } NEWLINE {
> if (parser->skip_stack &&
> parser->skip_stack->has_else)
> {
> @@ -376,7 +376,7 @@ control_line:
> if (parser->skip_stack)
> parser->skip_stack->has_else = true;
> }
> - } NEWLINE
> + }
> | HASH_ENDIF {
> _glcpp_parser_skip_stack_pop (parser, & @1);
> } NEWLINE
> @@ -1211,7 +1211,7 @@ glcpp_parser_create (const struct gl_extensions *extensions, gl_api api)
> parser->defines = hash_table_ctor (32, hash_table_string_hash,
> hash_table_string_compare);
> parser->active = NULL;
> - parser->lexing_if = 0;
> + parser->lexing_directive = 0;
> parser->space_tokens = 1;
> parser->newline_as_space = 0;
> parser->in_control_line = 0;
> diff --git a/src/glsl/glcpp/glcpp.h b/src/glsl/glcpp/glcpp.h
> index 79ccb23..b2654ff 100644
> --- a/src/glsl/glcpp/glcpp.h
> +++ b/src/glsl/glcpp/glcpp.h
> @@ -168,7 +168,7 @@ struct glcpp_parser {
> yyscan_t scanner;
> struct hash_table *defines;
> active_list_t *active;
> - int lexing_if;
> + int lexing_directive;
> int space_tokens;
> int newline_as_space;
> int in_control_line;
> diff --git a/src/glsl/glcpp/tests/103-garbage-after-else-0.c b/src/glsl/glcpp/tests/103-garbage-after-else-0.c
> new file mode 100644
> index 0000000..c460fea
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/103-garbage-after-else-0.c
> @@ -0,0 +1,3 @@
> +#if 0
> +#else garbage
> +#endif
> diff --git a/src/glsl/glcpp/tests/103-garbage-after-else-0.c.expected b/src/glsl/glcpp/tests/103-garbage-after-else-0.c.expected
> new file mode 100644
> index 0000000..f9f5f19
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/103-garbage-after-else-0.c.expected
> @@ -0,0 +1,4 @@
> +0:2(7): preprocessor error: syntax error, unexpected IDENTIFIER, expecting NEWLINE
> +0:1(7): preprocessor error: Unterminated #if
> +
> +
> diff --git a/src/glsl/glcpp/tests/103-garbage-after-else.c b/src/glsl/glcpp/tests/103-garbage-after-else.c
> deleted file mode 100644
> index c460fea..0000000
> --- a/src/glsl/glcpp/tests/103-garbage-after-else.c
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -#if 0
> -#else garbage
> -#endif
> diff --git a/src/glsl/glcpp/tests/103-garbage-after-else.c.expected b/src/glsl/glcpp/tests/103-garbage-after-else.c.expected
> deleted file mode 100644
> index f9f5f19..0000000
> --- a/src/glsl/glcpp/tests/103-garbage-after-else.c.expected
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -0:2(7): preprocessor error: syntax error, unexpected IDENTIFIER, expecting NEWLINE
> -0:1(7): preprocessor error: Unterminated #if
> -
> -
> diff --git a/src/glsl/glcpp/tests/123-garbage-after-else-1.c b/src/glsl/glcpp/tests/123-garbage-after-else-1.c
> new file mode 100644
> index 0000000..0b341a3
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/123-garbage-after-else-1.c
> @@ -0,0 +1,3 @@
> +#if 1
> +#else garbage
> +#endif
> diff --git a/src/glsl/glcpp/tests/123-garbage-after-else-1.c.expected b/src/glsl/glcpp/tests/123-garbage-after-else-1.c.expected
> new file mode 100644
> index 0000000..f9f5f19
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/123-garbage-after-else-1.c.expected
> @@ -0,0 +1,4 @@
> +0:2(7): preprocessor error: syntax error, unexpected IDENTIFIER, expecting NEWLINE
> +0:1(7): preprocessor error: Unterminated #if
> +
> +
> --
> 2.0.0
>
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
More information about the mesa-dev
mailing list