On Thursday, July 31, 2014 11:22:59 AM Carl Worth wrote:
> Strictly speaking, the GLSL specification only allows for the '#' to appear in
> a shader in two places:
>
> 1. To introduce a pre-processing directive
>
> In this case, the '#' must be the first character on a line after
> ignoring any comments and horizontal whitespace.
>
> 2. As part of the token-pasting operator
>
> In this case, the '#' must appear as part of the sequence "##" and
> can only appear in the replacement list of a macro definition.
>
> Since commit f062f0506a5b827667b7eb52136d8420b7e8113b, glcpp has been strict
> about '#' characters, such that any occurence outside of either of the above
> two cases would result in a syntax error of "Unexpected HASH_TOKEN" from the
> grammar.
>
> (Prior to that commit, these illegal '#' would be silently ignored and printed
> to stdout. And if it hadn't been for that commit, these illegal '#' would have
> triggered internal errors with later commits.)
>
> With this commit, glcpp relaxes its strictness and treats such appearances of
> '#' as just another piece of punctuation. This would make it legal to do
> something like:
>
> #define FOO # something here
>
> and then use "#ifdef FOO" and similar, (though if FOO were ever directly
> expanded, this should trigger a syntax error in the main GLSL compiler).
>
> Relaxing strict conformance compared to GLSL seems somewhat dubious here, but
> might be justified for a few reasons:
>
> 1. GCC's preprocessor allows this, (though C is not GLSL, of course)
>
> 2. glcpp is generally soft about illegal characters, (compare to
> non-ASCII characters which glcpp happily passes through, trusting
> the GLSL compiler to catch them).
>
> 3. At least one game in the wild (Reactive Quake) has been observed
> with a bogus '#' in it:
>
> #define USE_DISCARD#line 0
>
> Note that the shader (that we've seen) with this line does not ever
> *use* the USE_DISCARD macro. Note also that this shader is accepted
> by both Nvidia's OpenGL implementation as well as Mesa prior to
> the commit referenced above.
> ---
> src/glsl/glcpp/glcpp-lex.l | 12 +++++++-----
> src/glsl/glcpp/glcpp-parse.y | 1 +
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
> index c126850..cf517ad 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -270,15 +270,17 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
>
> /* If the '#' is the first non-whitespace, non-comment token on this
> * line, then it introduces a directive, switch to the <HASH> start
> - * condition.
> + * condition and return a HASH_TOKEN token.
> *
> - * Otherwise, this is just punctuation, so return the HASH_TOKEN
> - * token. */
> + * Otherwise, this is just punctuation, so just return a literal '#'
> + * token, (which will not be interpreted as a directive).
> + */
> if (parser->first_non_space_token_this_line) {
> BEGIN HASH;
> + RETURN_TOKEN_NEVER_SKIP (HASH_TOKEN);
> + } else {
> + RETURN_TOKEN ('#');
> }
> -
> - RETURN_TOKEN_NEVER_SKIP (HASH_TOKEN);
> }
>
> <HASH>version{HSPACE}+ {
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 4ee4110..b281b01 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -744,6 +744,7 @@ operator:
> | PASTE { $$ = PASTE; }
> | PLUS_PLUS { $$ = PLUS_PLUS; }
> | MINUS_MINUS { $$ = MINUS_MINUS; }
> +| '#' { $$ = '#'; }
> ;
>
> %%
I agree that this is pretty bogus. If the latest Reaction Quake still contains
this shader, we should probably report a bug. It's clearly broken - not what
the application developer intended - and silently accepting the broken shader
is not doing them any favors. But, we should probably also pass through '#' to
be safe.
How about emitting a warning in the RETURN_TOKEN ('#') case?
With or without that change (and I don't need to see a v2), this is:
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
