[Mesa-dev] [PATCH 1/4] glcpp: Implicitly resolve version after the first non-space/hash token.

Ian Romanick idr at freedesktop.org
Tue Mar 8 01:23:26 UTC 2016


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.romanick at intel.com>

> Fixes dEQP-GLES2.functional.shaders.preprocessor.predefined_macros.
> {gl_es_1_vertex,gl_es_1_fragment}.
> 
> Signed-off-by: Kenneth Graunke <kenneth at 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
> +
> +
> 



More information about the mesa-dev mailing list