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

[Erik Faye-Lund]

On Fri, Oct 11, 2013 at 10:48 PM, Ian Romanick <idr at freedesktop.org> wrote:
> Carl,
>
> Can you look at this patch and Erik's follow-up patch?  You (still) know
> the glcpp much better than any of the rest of us.
>
> (Carl is currently out of town, so I know his response will be slow...)
>

I guess "slow" doesn't mean *this* slow? :)

Should I resend the patch, with the #elseif fix squashed in?

> On 09/23/2013 01:35 PM, Erik Faye-Lund wrote:
>> The preprocessor currently eats multiple #else directives
>> int the same #if(def) ... #endif block. While I haven't been able
>> to find anything that explicitly disallows it, it's nonsensical
>> and should probably not be allowed.
>>
>> Add checks to reject the code.
>> ---
>>
>> I'm not entirely sure why parser->skip_stack can be NULL after
>> _glcpp_parser_skip_stack_change_if, but apparently it is for at
>> least one test.
>>
>> I'm also not entirely sure if this should be an error or a warning.
>> Thoughts?
>>
>>  src/glsl/glcpp/glcpp-parse.y                      | 13 ++++++++++++-
>>  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 ++++++++
>>  4 files changed, 27 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
>>
>> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
>> index 6eaa5f9..885c64d 100644
>> --- a/src/glsl/glcpp/glcpp-parse.y
>> +++ b/src/glsl/glcpp/glcpp-parse.y
>> @@ -332,7 +332,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);
>> @@ -2012,6 +2022,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;
>> +
>> +
>>
>