On 03/04/2016 07:33 PM, Kenneth Graunke wrote: > We resolved the implicit version directive when processing control lines, > such as #ifdef, to ensure any built-in macros exist. However, we failed > to resolve it when handling ordinary text. > > For example, > > int x = __VERSION__; > > should resolve __VERSION__ to 110, but since we never resolved the implicit > version, none of the built-in macros exist, so it was left as is. > > This also meant we allowed the following shader to slop through: > > 123 > #version 120 > > Nothing would cause the implicit version to take effect, so when we saw > the #version directive, we thought everything was peachy. > > This patch makes the lexer's per-token action resolve the implicit > version on the first non-space/newline/hash token that isn't part of > a #version directive, fulfilling the GLSL language spec: > > "The #version directive must occur in a shader before anything else, > except for comments and white space." > > Because we emit #version as HASH_TOKEN then VERSION_TOKEN, we have to > allow HASH_TOKEN to slop through as well, so we don't resolve the > implicit version as soon as we see the # character. However, this is > fine, because the parser's HASH_TOKEN NEWLINE rule does resolve the > version, disallowing cases like: > > # > #version 120 > > This patch also adds the above shaders as new glcpp tests.
Does any of this interfere with various workarounds that we have to allow mixed #version lines, #extension lines, and code? Assuming all that still works, this series is Reviewed-by: Ian Romanick <ian.d.roman...@intel.com> > Fixes dEQP-GLES2.functional.shaders.preprocessor.predefined_macros. > {gl_es_1_vertex,gl_es_1_fragment}. > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/compiler/glsl/glcpp/glcpp-lex.l | 8 ++++++++ > src/compiler/glsl/glcpp/glcpp.h | 1 + > src/compiler/glsl/glcpp/tests/144-implicit-version.c | 1 + > src/compiler/glsl/glcpp/tests/144-implicit-version.c.expected | 1 + > src/compiler/glsl/glcpp/tests/145-version-first.c | 2 ++ > src/compiler/glsl/glcpp/tests/145-version-first.c.expected | 3 +++ > src/compiler/glsl/glcpp/tests/146-version-first-hash.c | 2 ++ > src/compiler/glsl/glcpp/tests/146-version-first-hash.c.expected | 3 +++ > 8 files changed, 21 insertions(+) > create mode 100644 src/compiler/glsl/glcpp/tests/144-implicit-version.c > create mode 100644 > src/compiler/glsl/glcpp/tests/144-implicit-version.c.expected > create mode 100644 src/compiler/glsl/glcpp/tests/145-version-first.c > create mode 100644 src/compiler/glsl/glcpp/tests/145-version-first.c.expected > create mode 100644 src/compiler/glsl/glcpp/tests/146-version-first-hash.c > create mode 100644 > src/compiler/glsl/glcpp/tests/146-version-first-hash.c.expected > > diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l > b/src/compiler/glsl/glcpp/glcpp-lex.l > index fa9aa50..071918e 100644 > --- a/src/compiler/glsl/glcpp/glcpp-lex.l > +++ b/src/compiler/glsl/glcpp/glcpp-lex.l > @@ -120,6 +120,11 @@ void glcpp_set_column (int column_no , yyscan_t > yyscanner); > static int > glcpp_lex_update_state_per_token (glcpp_parser_t *parser, int token) > { > + if (token != NEWLINE && token != SPACE && token != HASH_TOKEN && > + !parser->lexing_version_directive) { > + glcpp_parser_resolve_implicit_version(parser); > + } > + > /* After the first non-space token in a line, we won't > * allow any '#' to introduce a directive. */ > if (token == NEWLINE) { > @@ -285,6 +290,7 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]? > <HASH>version{HSPACE}+ { > BEGIN INITIAL; > yyextra->space_tokens = 0; > + yyextra->lexing_version_directive = 1; > RETURN_STRING_TOKEN (VERSION_TOKEN); > } > > @@ -536,6 +542,7 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]? > } > yyextra->space_tokens = 1; > yyextra->lexing_directive = 0; > + yyextra->lexing_version_directive = 0; > yylineno++; > yycolumn = 0; > RETURN_TOKEN_NEVER_SKIP (NEWLINE); > @@ -546,6 +553,7 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]? > glcpp_error(yylloc, yyextra, "Unterminated comment"); > BEGIN DONE; /* Don't keep matching this rule forever. */ > yyextra->lexing_directive = 0; > + yyextra->lexing_version_directive = 0; > if (! parser->last_token_was_newline) > RETURN_TOKEN (NEWLINE); > } > diff --git a/src/compiler/glsl/glcpp/glcpp.h b/src/compiler/glsl/glcpp/glcpp.h > index 70aa14b..d87e6b7 100644 > --- a/src/compiler/glsl/glcpp/glcpp.h > +++ b/src/compiler/glsl/glcpp/glcpp.h > @@ -176,6 +176,7 @@ struct glcpp_parser { > struct hash_table *defines; > active_list_t *active; > int lexing_directive; > + int lexing_version_directive; > int space_tokens; > int last_token_was_newline; > int last_token_was_space; > diff --git a/src/compiler/glsl/glcpp/tests/144-implicit-version.c > b/src/compiler/glsl/glcpp/tests/144-implicit-version.c > new file mode 100644 > index 0000000..7bf72fc > --- /dev/null > +++ b/src/compiler/glsl/glcpp/tests/144-implicit-version.c > @@ -0,0 +1 @@ > +int x = __VERSION__; > diff --git a/src/compiler/glsl/glcpp/tests/144-implicit-version.c.expected > b/src/compiler/glsl/glcpp/tests/144-implicit-version.c.expected > new file mode 100644 > index 0000000..8c2dfd9 > --- /dev/null > +++ b/src/compiler/glsl/glcpp/tests/144-implicit-version.c.expected > @@ -0,0 +1 @@ > +int x = 110; > diff --git a/src/compiler/glsl/glcpp/tests/145-version-first.c > b/src/compiler/glsl/glcpp/tests/145-version-first.c > new file mode 100644 > index 0000000..f9fcfb0 > --- /dev/null > +++ b/src/compiler/glsl/glcpp/tests/145-version-first.c > @@ -0,0 +1,2 @@ > +123 > +#version 120 > diff --git a/src/compiler/glsl/glcpp/tests/145-version-first.c.expected > b/src/compiler/glsl/glcpp/tests/145-version-first.c.expected > new file mode 100644 > index 0000000..f4092b0 > --- /dev/null > +++ b/src/compiler/glsl/glcpp/tests/145-version-first.c.expected > @@ -0,0 +1,3 @@ > +0:2(1): preprocessor error: #version must appear on the first line > +123 > + > diff --git a/src/compiler/glsl/glcpp/tests/146-version-first-hash.c > b/src/compiler/glsl/glcpp/tests/146-version-first-hash.c > new file mode 100644 > index 0000000..14dbe96 > --- /dev/null > +++ b/src/compiler/glsl/glcpp/tests/146-version-first-hash.c > @@ -0,0 +1,2 @@ > +# > +#version 120 > diff --git a/src/compiler/glsl/glcpp/tests/146-version-first-hash.c.expected > b/src/compiler/glsl/glcpp/tests/146-version-first-hash.c.expected > new file mode 100644 > index 0000000..2872090 > --- /dev/null > +++ b/src/compiler/glsl/glcpp/tests/146-version-first-hash.c.expected > @@ -0,0 +1,3 @@ > +0:1(3): preprocessor error: #version must appear on the first line > + > + > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev