[Mesa-dev] [PATCH] glsl: fixer lexer for unreachable defines

[Timothy Arceri]

On 03/09/18 22:00, Eero Tamminen wrote:
> Hi,
> 
> Seems a lot of replicated code to deal with regression from few line 
> line patch,

That's because the previous patch didn't cause the regression. It fixed 
another bug which happened to be hiding this problem we are seeing now.

> but at least it fixed the failing GfxBench ALU2 test. :-)
> 
> Tested-By: Eero Tamminen <eero.t.tamminen at intel.com>
> 
> 
>      - Eero
> 
> On 01.09.2018 16:57, Timothy Arceri wrote:
>> If we have something like:
>>
>>     #ifdef NOT_DEFINED
>>     #define A_MACRO(x) \
>>     if (x)
>>     #endif
>>
>> The # on the #define is not skipped but the define itself is so
>> this then gets recognised as #if.
>>
>> Until 28a3731e3f this didn't happen because we ended up in
>> <HASH>{NONSPACE} where BEGIN INITIAL was called stopping the
>> problem from happening.
>>
>> This change makes sure we never call RETURN_TOKEN_NEVER_SKIP for
>> if/else/endif when processing a define.
>>
>> Cc: Ian Romanick <idr at freedesktop.org>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107772
>> ---
>>   src/compiler/glsl/glcpp/glcpp-lex.l | 60 ++++++++++++++++++-----------
>>   src/compiler/glsl/glcpp/glcpp.h     |  1 +
>>   2 files changed, 38 insertions(+), 23 deletions(-)
>>
>> diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l 
>> b/src/compiler/glsl/glcpp/glcpp-lex.l
>> index fe5845acd4e..f7003da0cc8 100644
>> --- a/src/compiler/glsl/glcpp/glcpp-lex.l
>> +++ b/src/compiler/glsl/glcpp/glcpp-lex.l
>> @@ -289,6 +289,7 @@ HEXADECIMAL_INTEGER    0[xX][0-9a-fA-F]+[uU]?
>>            * token. */
>>       if (parser->first_non_space_token_this_line) {
>>           BEGIN HASH;
>> +        yyextra->in_define = false;
>>       }
>>       RETURN_TOKEN_NEVER_SKIP (HASH_TOKEN);
>> @@ -336,43 +337,55 @@ HEXADECIMAL_INTEGER    0[xX][0-9a-fA-F]+[uU]?
>>       /* For the pre-processor directives, we return these tokens
>>        * even when we are otherwise skipping. */
>>   <HASH>ifdef {
>> -    BEGIN INITIAL;
>> -    yyextra->lexing_directive = 1;
>> -    yyextra->space_tokens = 0;
>> -    RETURN_TOKEN_NEVER_SKIP (IFDEF);
>> +    if (!yyextra->in_define) {
>> +        BEGIN INITIAL;
>> +        yyextra->lexing_directive = 1;
>> +        yyextra->space_tokens = 0;
>> +        RETURN_TOKEN_NEVER_SKIP (IFDEF);
>> +    }
>>   }
>>   <HASH>ifndef {
>> -    BEGIN INITIAL;
>> -    yyextra->lexing_directive = 1;
>> -    yyextra->space_tokens = 0;
>> -    RETURN_TOKEN_NEVER_SKIP (IFNDEF);
>> +    if (!yyextra->in_define) {
>> +        BEGIN INITIAL;
>> +        yyextra->lexing_directive = 1;
>> +        yyextra->space_tokens = 0;
>> +        RETURN_TOKEN_NEVER_SKIP (IFNDEF);
>> +    }
>>   }
>>   <HASH>if/[^_a-zA-Z0-9] {
>> -    BEGIN INITIAL;
>> -    yyextra->lexing_directive = 1;
>> -    yyextra->space_tokens = 0;
>> -    RETURN_TOKEN_NEVER_SKIP (IF);
>> +    if (!yyextra->in_define) {
>> +        BEGIN INITIAL;
>> +        yyextra->lexing_directive = 1;
>> +        yyextra->space_tokens = 0;
>> +        RETURN_TOKEN_NEVER_SKIP (IF);
>> +    }
>>   }
>>   <HASH>elif/[^_a-zA-Z0-9] {
>> -    BEGIN INITIAL;
>> -    yyextra->lexing_directive = 1;
>> -    yyextra->space_tokens = 0;
>> -    RETURN_TOKEN_NEVER_SKIP (ELIF);
>> +    if (!yyextra->in_define) {
>> +        BEGIN INITIAL;
>> +        yyextra->lexing_directive = 1;
>> +        yyextra->space_tokens = 0;
>> +        RETURN_TOKEN_NEVER_SKIP (ELIF);
>> +    }
>>   }
>>   <HASH>else {
>> -    BEGIN INITIAL;
>> -    yyextra->space_tokens = 0;
>> -    RETURN_TOKEN_NEVER_SKIP (ELSE);
>> +    if (!yyextra->in_define) {
>> +        BEGIN INITIAL;
>> +        yyextra->space_tokens = 0;
>> +        RETURN_TOKEN_NEVER_SKIP (ELSE);
>> +    }
>>   }
>>   <HASH>endif {
>> -    BEGIN INITIAL;
>> -    yyextra->space_tokens = 0;
>> -    RETURN_TOKEN_NEVER_SKIP (ENDIF);
>> +    if (!yyextra->in_define) {
>> +        BEGIN INITIAL;
>> +        yyextra->space_tokens = 0;
>> +        RETURN_TOKEN_NEVER_SKIP (ENDIF);
>> +    }
>>   }
>>   <HASH>error[^\r\n]* {
>> @@ -399,7 +412,8 @@ HEXADECIMAL_INTEGER    0[xX][0-9a-fA-F]+[uU]?
>>        *      and not whitespace). This will generate an error.
>>        */
>>   <HASH>define{HSPACE}* {
>> -    if (! parser->skipping) {
>> +    yyextra->in_define = true;
>> +    if (!parser->skipping) {
>>           BEGIN DEFINE;
>>           yyextra->space_tokens = 0;
>>           RETURN_TOKEN (DEFINE_TOKEN);
>> diff --git a/src/compiler/glsl/glcpp/glcpp.h 
>> b/src/compiler/glsl/glcpp/glcpp.h
>> index c7e382ed30c..e786b24b132 100644
>> --- a/src/compiler/glsl/glcpp/glcpp.h
>> +++ b/src/compiler/glsl/glcpp/glcpp.h
>> @@ -197,6 +197,7 @@ struct glcpp_parser {
>>       int first_non_space_token_this_line;
>>       int newline_as_space;
>>       int in_control_line;
>> +    bool in_define;
>>       int paren_count;
>>       int commented_newlines;
>>       skip_node_t *skip_stack;
>>
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev