[Mesa-dev] [PATCH] glcpp: fix #undef to match latest spec update and GLSLang implementation

[Iago Toral Quiroga]

GLSL ES spec includes the following:

   "It is an error to undefine or to redefine a built-in
    (pre-defined) macro name."

But desktop GLSL doesn't. This has sparked some discussion
in Khronos, and the final conclusion was to update the
GLSL 4.50 spec to include the following:

   "By convention, all macro names containing two consecutive
    underscores ( __ ) are reserved for use by underlying
    software layers.  Defining or undefining such a name in a
    shader does not itself result in an error, but may result
    in unintended behaviors that stem from having multiple
    definitions of the same name.  All macro names prefixed
    with “GL_” (“GL” followed by a single underscore) are also
    reserved, and defining or undefining such a name results in
    a compile-time error."

In other words, undefining GL_* names should be an error, but
undefining other names with a double underscore in them is
not strictly prohibited in desktop GLSL.

This patch fixes the preprocessor to apply these rules,
following exactly the implementation already present
in GLSLang. This fixes some tests in CTS.

Kronos bug:
https://cvs.khronos.org/bugzilla/show_bug.cgi?id=16003

Fixes:
KHR-GL45.shaders.preprocessor.definitions.undefine_core_profile_vertex
KHR-GL45.shaders.preprocessor.definitions.undefine_core_profile_fragment
---
 src/compiler/glsl/glcpp/glcpp-parse.y | 45 ++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y b/src/compiler/glsl/glcpp/glcpp-parse.y
index fe211a0..f1719f9 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -287,24 +287,41 @@ control_line_success:
                  * The GLSL ES 1.00 spec does not contain this text, but
                  * dEQP's preprocess test in GLES2 checks for it.
                  *
-                 * Section 3.3 (Preprocessor) of the GLSL 1.30 spec says:
+                 * Section 3.3 (Preprocessor) revision 7, of the GLSL 4.50
+                 * spec says:
                  *
-                 *    #define and #undef functionality are defined as is
-                 *    standard for C++ preprocessors for macro definitions
-                 *    both with and without macro parameters.
+                 *    By convention, all macro names containing two consecutive
+                 *    underscores ( __ ) are reserved for use by underlying
+                 *    software layers. Defining or undefining such a name
+                 *    in a shader does not itself result in an error, but may
+                 *    result in unintended behaviors that stem from having
+                 *    multiple definitions of the same name. All macro names
+                 *    prefixed with "GL_" (...) are also reseved, and defining
+                 *    such a name results in a compile-time error.
                  *
-                 * At least as far as I can tell GCC allow '#undef __FILE__'.
-                 * Furthermore, there are desktop OpenGL conformance tests
-                 * that expect '#undef __VERSION__' and '#undef
-                 * GL_core_profile' to work.
+                 * The code below implements the same checks as GLSLang.
                  */
-		if (parser->is_gles &&
-                    (strcmp("__LINE__", $3) == 0
-                     || strcmp("__FILE__", $3) == 0
-                     || strcmp("__VERSION__", $3) == 0
-                     || strncmp("GL_", $3, 3) == 0))
+		if (strncmp("GL_", $3, 3) == 0)
 			glcpp_error(& @1, parser, "Built-in (pre-defined)"
-				    " macro names cannot be undefined.");
+				    " names beginning with GL_ cannot be undefined.");
+		else if (strstr($3, "__") != NULL) {
+			if (parser->is_gles
+			    && parser->version >= 300
+			    && (strcmp("__LINE__", $3) == 0
+				|| strcmp("__FILE__", $3) == 0
+				|| strcmp("__VERSION__", $3) == 0)) {
+				glcpp_error(& @1, parser, "Built-in (pre-defined)"
+					    " names cannot be undefined.");
+			} else if (parser->is_gles && parser->version <= 300) {
+				glcpp_error(& @1, parser,
+					    " names containing consecutive underscores"
+					    " are reserved.");
+			} else {
+				glcpp_warning(& @1, parser,
+					      " names containing consecutive underscores"
+					      " are reserved.");
+			}
+		}
 
 		entry = _mesa_hash_table_search (parser->defines, $3);
 		if (entry) {
-- 
2.9.3