This didn't get any reviews, any takers? It fixes a couple of CTS tests so it would be good to have it merged.
Iago On Tue, 2017-05-30 at 13:25 +0200, Iago Toral Quiroga wrote: > 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) { _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev