On 24/04/18 02:04, Vlad Golovkin wrote:
2018-04-23 3:53 GMT+03:00 Timothy Arceri <tarc...@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
+
+
+


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to