glsl/glcpp: Add short-circuiting for || and && in #if/#elif for OpenGL ES.
authorCarl Worth <cworth@cworth.org>
Fri, 13 Jun 2014 21:54:46 +0000 (14:54 -0700)
committerCarl Worth <cworth@cworth.org>
Wed, 9 Jul 2014 19:05:13 +0000 (12:05 -0700)
The GLSL ES Specification 3.00.4 says:

#if, #ifdef, #ifndef, #else, #elif, and #endif are defined to operate
        as for C++ except for the following:
...
• Undefined identifiers not consumed by the defined operator do not
  default to '0'. Use of such identifiers causes an error.

[Page 11 (page 127 of the PDF file)]

as well as:

The semantics of applying operators in the preprocessor match those
standard in the C++ preprocessor with the following exceptions:

• The 2nd operand in a logical and ('&&') operation is evaluated if
  and only if the 1st operand evaluates to non-zero.

• The 2nd operand in a logical or ('||') operation is evaluated if
  and only if the 1st operand evaluates to zero.

If an operand is not evaluated, the presence of undefined identifiers
in the operand will not cause an error.

(Note that neither of these deviations from C++ preprocessor behavior apply to
non-ES GLSL, at least as of specfication version 4.30.6).

The first portion of this, (generating an error for an undefined macro in an
(short-circuiting to squelch errors), was not implemented previously, but is
implemented in this commit.

A test is added for "make check" to ensure this behavior.

Note: The change as implemented does make the error message a bit less
precise, (it just states that an undefined macro was encountered, but not the
name of the macro).

This commit fixes the following Khronos GLES3 conformance test:

undefined_identifiers.valid_undefined_identifier_1_vertex
undefined_identifiers.valid_undefined_identifier_1_fragment
undefined_identifiers.valid_undefined_identifier_2_vertex
undefined_identifiers.valid_undefined_identifier_2_fragment

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
src/glsl/glcpp/glcpp-parse.y
src/glsl/glcpp/glcpp.h
src/glsl/glcpp/tests/125-es-short-circuit-undefined.c [new file with mode: 0644]
src/glsl/glcpp/tests/125-es-short-circuit-undefined.c.expected [new file with mode: 0644]

index 82e310270d6324b5dff100926c50f733a9d51429..68e4ace012ee85f0412412d7630027f42cad3729 100644 (file)
@@ -166,7 +166,8 @@ add_builtin_define(glcpp_parser_t *parser, const char *name, int value);
 %expect 0
 %token COMMA_FINAL DEFINED ELIF_EXPANDED HASH HASH_DEFINE FUNC_IDENTIFIER OBJ_IDENTIFIER HASH_ELIF HASH_ELSE HASH_ENDIF HASH_IF HASH_IFDEF HASH_IFNDEF HASH_LINE HASH_UNDEF HASH_VERSION IDENTIFIER IF_EXPANDED INTEGER INTEGER_STRING LINE_EXPANDED NEWLINE OTHER PLACEHOLDER SPACE
 %token PASTE
-%type <ival> expression INTEGER operator SPACE integer_constant
+%type <ival> INTEGER operator SPACE integer_constant
+%type <expression_value> expression
 %type <str> IDENTIFIER FUNC_IDENTIFIER OBJ_IDENTIFIER INTEGER_STRING OTHER
 %type <string_list> identifier_list
 %type <token> preprocessing_token conditional_token
@@ -216,10 +217,14 @@ line:
 
 expanded_line:
        IF_EXPANDED expression NEWLINE {
-               _glcpp_parser_skip_stack_push_if (parser, & @1, $2);
+               if (parser->is_gles && $2.has_undefined)
+                       glcpp_error(& @1, parser, "undefined macro in expression (illegal in GLES)");
+               _glcpp_parser_skip_stack_push_if (parser, & @1, $2.value);
        }
 |      ELIF_EXPANDED expression NEWLINE {
-               _glcpp_parser_skip_stack_change_if (parser, & @1, "elif", $2);
+               if (parser->is_gles && $2.has_undefined)
+                       glcpp_error(& @1, parser, "undefined macro in expression (illegal in GLES)");
+               _glcpp_parser_skip_stack_change_if (parser, & @1, "elif", $2.value);
        }
 |      LINE_EXPANDED integer_constant NEWLINE {
                parser->has_new_line_number = 1;
@@ -412,87 +417,128 @@ integer_constant:
        }
 
 expression:
-       integer_constant
+       integer_constant {
+               $$.value = $1;
+               $$.has_undefined = false;
+       }
 |      IDENTIFIER {
+               $$.value = 0;
                if (parser->is_gles)
-                       glcpp_error(& @1, parser, "undefined macro %s in expression (illegal in GLES)", $1);
-               $$ = 0;
+                       $$.has_undefined = true;
+               else
+                       $$.has_undefined = false;
        }
 |      expression OR expression {
-               $$ = $1 || $3;
+               $$.value = $1.value || $3.value;
+
+               /* Short-circuit: Only flag undefined from right side
+                * if left side evaluates to false.
+                */
+               if ($1.value)
+                       $$.has_undefined = $1.has_undefined;
+               else
+                       $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression AND expression {
-               $$ = $1 && $3;
+               $$.value = $1.value && $3.value;
+
+               /* Short-circuit: Only flag undefined from right-side
+                * if left side evaluates to true.
+                */
+               if ($1.value)
+                       $$.has_undefined = $1.has_undefined || $3.has_undefined;
+               else
+                       $$.has_undefined = $1.has_undefined;
        }
 |      expression '|' expression {
-               $$ = $1 | $3;
+               $$.value = $1.value | $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '^' expression {
-               $$ = $1 ^ $3;
+               $$.value = $1.value ^ $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '&' expression {
-               $$ = $1 & $3;
+               $$.value = $1.value & $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression NOT_EQUAL expression {
-               $$ = $1 != $3;
+               $$.value = $1.value != $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression EQUAL expression {
-               $$ = $1 == $3;
+               $$.value = $1.value == $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression GREATER_OR_EQUAL expression {
-               $$ = $1 >= $3;
+               $$.value = $1.value >= $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression LESS_OR_EQUAL expression {
-               $$ = $1 <= $3;
+               $$.value = $1.value <= $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '>' expression {
-               $$ = $1 > $3;
+               $$.value = $1.value > $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '<' expression {
-               $$ = $1 < $3;
+               $$.value = $1.value < $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression RIGHT_SHIFT expression {
-               $$ = $1 >> $3;
+               $$.value = $1.value >> $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression LEFT_SHIFT expression {
-               $$ = $1 << $3;
+               $$.value = $1.value << $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '-' expression {
-               $$ = $1 - $3;
+               $$.value = $1.value - $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '+' expression {
-               $$ = $1 + $3;
+               $$.value = $1.value + $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '%' expression {
-               if ($3 == 0) {
+               if ($3.value == 0) {
                        yyerror (& @1, parser,
                                 "zero modulus in preprocessor directive");
                } else {
-                       $$ = $1 % $3;
+                       $$.value = $1.value % $3.value;
                }
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '/' expression {
-               if ($3 == 0) {
+               if ($3.value == 0) {
                        yyerror (& @1, parser,
                                 "division by 0 in preprocessor directive");
                } else {
-                       $$ = $1 / $3;
+                       $$.value = $1.value / $3.value;
                }
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      expression '*' expression {
-               $$ = $1 * $3;
+               $$.value = $1.value * $3.value;
+               $$.has_undefined = $1.has_undefined || $3.has_undefined;
        }
 |      '!' expression %prec UNARY {
-               $$ = ! $2;
+               $$.value = ! $2.value;
+               $$.has_undefined = $2.has_undefined;
        }
 |      '~' expression %prec UNARY {
-               $$ = ~ $2;
+               $$.value = ~ $2.value;
+               $$.has_undefined = $2.has_undefined;
        }
 |      '-' expression %prec UNARY {
-               $$ = - $2;
+               $$.value = - $2.value;
+               $$.has_undefined = $2.has_undefined;
        }
 |      '+' expression %prec UNARY {
-               $$ = + $2;
+               $$.value = + $2.value;
+               $$.has_undefined = $2.has_undefined;
        }
 |      '(' expression ')' {
                $$ = $2;
index b2654ffbed26906438dcfaa33a2aa4124fb5aa0d..9783210b7080849b26b44a9f93e3fdf80f95bd0c 100644 (file)
 
 /* Some data types used for parser values. */
 
+typedef struct expression_value {
+       intmax_t value;
+       bool has_undefined;
+} expression_value_t;
+   
+
 typedef struct string_node {
        const char *str;
        struct string_node *next;
@@ -53,6 +59,7 @@ typedef struct token_list token_list_t;
 typedef union YYSTYPE
 {
        intmax_t ival;
+       expression_value_t expression_value;
        char *str;
        string_list_t *string_list;
        token_t *token;
diff --git a/src/glsl/glcpp/tests/125-es-short-circuit-undefined.c b/src/glsl/glcpp/tests/125-es-short-circuit-undefined.c
new file mode 100644 (file)
index 0000000..4ee29f6
--- /dev/null
@@ -0,0 +1,27 @@
+/* For GLSL in OpenGL ES, an undefined macro appearing in an #if or #elif
+ * expression, (other than as an argument to defined) is an error.
+ *
+ * Except in the case of a short-circuiting && or || operator, where the
+ * specification explicitly mandates that there be no error.
+ */
+#version 300 es
+
+/* These yield errors */
+#if NOT_DEFINED
+#endif
+
+#if 0
+#elif ALSO_NOT_DEFINED
+#endif
+
+/* But these yield no errors */
+#if 1 || STILL_NOT_DEFINED
+Success
+#endif
+
+#if 0
+#elif 0 && WILL_ANYONE_DEFINE_ANYTHING
+#else
+More success
+#endif
+
diff --git a/src/glsl/glcpp/tests/125-es-short-circuit-undefined.c.expected b/src/glsl/glcpp/tests/125-es-short-circuit-undefined.c.expected
new file mode 100644 (file)
index 0000000..a52dae8
--- /dev/null
@@ -0,0 +1,30 @@
+0:10(16): preprocessor error: undefined macro in expression (illegal in GLES)
+0:14(23): preprocessor error: undefined macro in expression (illegal in GLES)
+
+
+
+
+
+#version 300 es
+
+
+
+
+
+
+
+
+
+
+
+Success
+
+
+
+
+
+More success
+
+
+