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

[Timothy Arceri]

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.

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
> +
> +
> +
>