[Mesa-dev] [PATCH] glsl/glcpp: Handle hex constants with 0X prefix

[Timothy Arceri]

On 24/04/18 02:04, Vlad Golovkin wrote:
> 2018-04-23 3:53 GMT+03:00 Timothy Arceri <tarceri at itsqueeze.com>:
>>
>>
>> On 20/04/18 06:08, Vlad Golovkin wrote:
>>>
>>> GLSL 4.6 spec describes hex constant as:
>>>
>>> hexadecimal-constant:
>>>       0x hexadecimal-digit
>>>       0X hexadecimal-digit
>>>       hexadecimal-constant hexadecimal-digit
>>>
>>> Right now if you have a shader with the following structure:
>>>
>>>       #if 0X1 // or any hex number with the 0X prefix
>>>       // some code
>>>       #endif
>>>
>>> the code between #if and #endif gets removed because the checking is
>>> performed
>>> only for "0x" prefix which results in strtoll being called with the base 8
>>> and
>>> after encountering the 'X' char the strtoll returns 0. Letting strtoll
>>> detect
>>> the base makes this limitation go away and also makes code easier to read.
>>
>>
>> The problem is strtoll supports much more than what GLSL allows.
>>
>>>
>>> Also added 1 test for uppercase hex prefix.
>>> ---
>>>    src/compiler/glsl/glcpp/glcpp-parse.y                            | 9
>>> ++-------
>>>    src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c   | 5
>>> +++++
>>>    .../glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected   | 5
>>> +++++
>>>    3 files changed, 12 insertions(+), 7 deletions(-)
>>>    create mode 100644
>>> src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>>>    create mode 100644
>>> src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>>>
>>> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y
>>> b/src/compiler/glsl/glcpp/glcpp-parse.y
>>> index ccb3aa18d3..d83f99f1c7 100644
>>> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
>>> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
>>> @@ -462,13 +462,8 @@ control_line_error:
>>>      integer_constant:
>>>          INTEGER_STRING {
>>> -               if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {
>>
>>
>>
>> As per my coment above strtoll supports much more than what GLSL allows.
>> Please just add strncmp($1, "0X", 2) == 0 to the if above.
> 
> Flex and Bison is not my strongest suit, so correct me if I am wrong,
> but it seems that INTEGER_STRING can't contain leading whitespace
> and/or leading plus/minus that strtoll supports.
> As for the multitude of bases that strtoll supports, it would ignore
> them and just choose between 8, 10 and 16 when base = 0. I think I
> should have made it more clear in the code comment.

Your right I missed that when reading the docs. I'll add something to 
the comment and push your patch. Thanks!

> 
>>
>> That patch would have my r-b. If you send that fixed up patch I can push it
>> for you. Thanks for looking into this.
>>
>>
>>
>>> -                       $$ = strtoll ($1 + 2, NULL, 16);
>>> -               } else if ($1[0] == '0') {
>>> -                       $$ = strtoll ($1, NULL, 8);
>>> -               } else {
>>> -                       $$ = strtoll ($1, NULL, 10);
>>> -               }
>>> +               /* let strtoll detect the base */
>>> +               $$ = strtoll ($1, NULL, 0);
>>>          }
>>>    |     INTEGER {
>>>                  $$ = $1;
>>> diff --git
>>> a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>>> b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>>> new file mode 100644
>>> index 0000000000..1be9b28eb7
>>> --- /dev/null
>>> +++ b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c
>>> @@ -0,0 +1,5 @@
>>> +#if 0x1234abcd == 0X1234abcd
>>> +success
>>> +#else
>>> +failure
>>> +#endif
>>> diff --git
>>> a/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>>> b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>>> new file mode 100644
>>> index 0000000000..4cf250f6bb
>>> --- /dev/null
>>> +++
>>> b/src/compiler/glsl/glcpp/tests/149-hex-const-uppercase-prefix.c.expected
>>> @@ -0,0 +1,5 @@
>>> +
>>> +success
>>> +
>>> +
>>> +
>>>
>>