[Mesa-dev] [PATCH v2] glcpp: error on multiple #else/#elif directives

[Ian Romanick]

On 12/17/2013 07:37 AM, Erik Faye-Lund wrote:
> The preprocessor currently accepts multiple else/elif-groups
> per if-section. The GLSL-preprocessor is defined by the C++
> specification, which defines the following parse-rule:
> 
> if-section:
> 	if-group elif-groups(opt) else-group(opt) endif-line
> 
> This clearly only allows a single else-group, that has to come
> after any elif-groups.
> 
> So let's modify the code to follow the specification. Add test
> to prevent regressions.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
Cc: 10.0 <mesa-stable at lists.freedesktop.org>

> ---
> 
> Here's a resend of an older patch (original
> <1379968503-30246-1-git-send-email-kusmabite at gmail.com>), this time
> with a better commit message, and a similar treatment for elif-after-else.
> 
>  src/glsl/glcpp/glcpp-parse.y                       | 23 +++++++++++++++++++++-
>  src/glsl/glcpp/glcpp.h                             |  1 +
>  src/glsl/glcpp/tests/118-multiple-else.c           |  6 ++++++
>  src/glsl/glcpp/tests/118-multiple-else.c.expected  |  8 ++++++++
>  src/glsl/glcpp/tests/119-elif-after-else.c         |  6 ++++++
>  .../glcpp/tests/119-elif-after-else.c.expected     |  8 ++++++++
>  6 files changed, 51 insertions(+), 1 deletion(-)
>  create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c
>  create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c.expected
>  create mode 100644 src/glsl/glcpp/tests/119-elif-after-else.c
>  create mode 100644 src/glsl/glcpp/tests/119-elif-after-else.c.expected
> 
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 7edc274..451b728 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -310,6 +310,11 @@ control_line:
>  			_glcpp_parser_expand_and_lex_from (parser,
>  							   ELIF_EXPANDED, $2);
>  		}
> +		else if (parser->skip_stack &&
> +		    parser->skip_stack->has_else)
> +		{
> +			glcpp_error(& @1, parser, "#elif after #else");
> +		}
>  		else
>  		{
>  			_glcpp_parser_skip_stack_change_if (parser, & @1,
> @@ -324,6 +329,11 @@ control_line:
>  		{
>  			glcpp_error(& @1, parser, "#elif with no expression");
>  		}
> +		else if (parser->skip_stack &&
> +		    parser->skip_stack->has_else)
> +		{
> +			glcpp_error(& @1, parser, "#elif after #else");
> +		}
>  		else
>  		{
>  			_glcpp_parser_skip_stack_change_if (parser, & @1,
> @@ -332,7 +342,17 @@ control_line:
>  		}
>  	}
>  |	HASH_ELSE {
> -		_glcpp_parser_skip_stack_change_if (parser, & @1, "else", 1);
> +		if (parser->skip_stack &&
> +		    parser->skip_stack->has_else)
> +		{
> +			glcpp_error(& @1, parser, "multiple #else");
> +		}
> +		else
> +		{
> +			_glcpp_parser_skip_stack_change_if (parser, & @1, "else", 1);
> +			if (parser->skip_stack)
> +				parser->skip_stack->has_else = true;
> +		}
>  	} NEWLINE
>  |	HASH_ENDIF {
>  		_glcpp_parser_skip_stack_pop (parser, & @1);
> @@ -2024,6 +2044,7 @@ _glcpp_parser_skip_stack_push_if (glcpp_parser_t *parser, YYLTYPE *loc,
>  		node->type = SKIP_TO_ENDIF;
>  	}
>  
> +	node->has_else = false;
>  	node->next = parser->skip_stack;
>  	parser->skip_stack = node;
>  }
> diff --git a/src/glsl/glcpp/glcpp.h b/src/glsl/glcpp/glcpp.h
> index 8aaa551..ccae96c 100644
> --- a/src/glsl/glcpp/glcpp.h
> +++ b/src/glsl/glcpp/glcpp.h
> @@ -153,6 +153,7 @@ typedef enum skip_type {
>  
>  typedef struct skip_node {
>  	skip_type_t type;
> +	bool has_else;
>  	YYLTYPE loc; /* location of the initial #if/#elif/... */
>  	struct skip_node *next;
>  } skip_node_t;
> diff --git a/src/glsl/glcpp/tests/118-multiple-else.c b/src/glsl/glcpp/tests/118-multiple-else.c
> new file mode 100644
> index 0000000..62ad49c
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/118-multiple-else.c
> @@ -0,0 +1,6 @@
> +#if 0
> +#else
> +int foo;
> +#else
> +int bar;
> +#endif
> diff --git a/src/glsl/glcpp/tests/118-multiple-else.c.expected b/src/glsl/glcpp/tests/118-multiple-else.c.expected
> new file mode 100644
> index 0000000..eaec481
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/118-multiple-else.c.expected
> @@ -0,0 +1,8 @@
> +0:4(1): preprocessor error: multiple #else
> +
> +
> +int foo;
> +
> +int bar;
> +
> +
> diff --git a/src/glsl/glcpp/tests/119-elif-after-else.c b/src/glsl/glcpp/tests/119-elif-after-else.c
> new file mode 100644
> index 0000000..9b9e923
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/119-elif-after-else.c
> @@ -0,0 +1,6 @@
> +#if 0
> +#else
> +int foo;
> +#elif 0
> +int bar;
> +#endif
> diff --git a/src/glsl/glcpp/tests/119-elif-after-else.c.expected b/src/glsl/glcpp/tests/119-elif-after-else.c.expected
> new file mode 100644
> index 0000000..33f0513
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/119-elif-after-else.c.expected
> @@ -0,0 +1,8 @@
> +0:4(1): preprocessor error: #elif after #else
> +
> +
> +int foo;
> +
> +int bar;
> +
> +
>