[Mesa-dev] [PATCH] glsl: Fix glcpp to properly lex entire "preprocessing numbers"

[Anuj Phogat]

On Thu, Jun 12, 2014 at 5:18 PM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
> On Thu, Jun 12, 2014 at 3:13 PM, Carl Worth <cworth at cworth.org> wrote:
>>
>> The preprocessor defines a notions of a "preprocessing number" that
>> starts with either a digit or a decimal point, and continues with zero
>> or more of digits, decimal points, identifier characters, or the sign
>> symbols, ('-' and '+').
>>
>> Prior to this change, preprocessing numbers were lexed as some
>> combination of OTHER and IDENTIFIER tokens. This had the problem of
>> causing undesired macro expansion in some cases.
>>
>> We add tests to ensure that the undesired macro expansion does not
>> happen in cases such as:
>>
>>         #define e +1
>>         #define xyz -2
>>
>>         int n = 1e;
>>         int p = 1xyz;
>>
>> In either case these macro definitions have no effect after this
>> change, so that the numeric literals, (whether valid or not), will be
>> passed on as-is from the preprocessor to the compiler proper.
>>
>> This fixes at least the following Khronos GLES3 CTS test:
>>
>> preprocessor.basic.correct_phases_fragment
>> ---
>>  src/glsl/glcpp/glcpp-lex.l                                | 6 ++++++
>>  src/glsl/glcpp/tests/124-preprocessing-numbers.c          | 8 ++++++++
>>  src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected | 9 +++++++++
>>  3 files changed, 23 insertions(+)
>>  create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c
>>  create mode 100644 src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
>>
>> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
>> index d5fb087..4dbaa9e 100644
>> --- a/src/glsl/glcpp/glcpp-lex.l
>> +++ b/src/glsl/glcpp/glcpp-lex.l
>> @@ -91,6 +91,7 @@ DIGITS                        [0-9][0-9]*
>>  DECIMAL_INTEGER                [1-9][0-9]*[uU]?
>>  OCTAL_INTEGER          0[0-7]*[uU]?
>>  HEXADECIMAL_INTEGER    0[xX][0-9a-fA-F]+[uU]?
>> +PP_NUMBER              [0-9.][-+._a-zA-Z0-9]*
It causes '2.0-MACRO' to incorrectly lexed as a preprocessing number.
Around 30 GLES3 CTS tests failed with this patch.

I think using below definition of preprocessing numbers should work fine:
PP_NUMBER                   \.?[0-9]([._a-zA-Z0-9]|[eEpP][+-])*

Used below link for definition of a preprocessing number:
https://gcc.gnu.org/onlinedocs/cpp/Tokenization.html

>>
>>  %%
>>
>> @@ -339,6 +340,11 @@ HEXADECIMAL_INTEGER        0[xX][0-9a-fA-F]+[uU]?
>>         return OTHER;
>>  }
>>
>> +{PP_NUMBER} {
>> +       yylval->str = ralloc_strdup (yyextra, yytext);
>> +       return OTHER;
>> +}
>> +
>>  {HSPACE} {
>>         if (yyextra->space_tokens) {
>>                 return SPACE;
>> diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
>> new file mode 100644
>> index 0000000..3743c69
>> --- /dev/null
>> +++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c
>> @@ -0,0 +1,8 @@
>> +#define e +1
>> +#define xyz -2
>> +
>> +/* The following are "preprocessing numbers" and should not trigger macro
>> + * expansion. */
>> +int n = 1e;
>> +int p = 1xyz;
>> +
>> diff --git a/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
>> new file mode 100644
>> index 0000000..951581e
>> --- /dev/null
>> +++ b/src/glsl/glcpp/tests/124-preprocessing-numbers.c.expected
>> @@ -0,0 +1,9 @@
>> +
>> +
>> +
>> +
>> +
>> +int n = 1e;
>> +int p = 1xyz;
>> +
>> +
>> --
>> 2.0.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>