Ian Romanick <i...@freedesktop.org> writes: > On 07/31/2014 12:14 AM, Michel Dänzer wrote: >> >> FYI, this change broke the game Reaction Quake, see the failure output >> below. I don't know if this is a bug in this change or in the game, so >> I'm reporting it here.
Hi Michel,
Thanks very much for reporting this failure. It's certainly an
interesting one.
>> #define USE_DISCARD#line 0
>
> Something is seriously wrong here.
The above line in the shader does seem to be what is triggering the
error you noticed:
>> 0:46(20): preprocessor error: syntax error, unexpected HASH_TOKEN, expecting
>> NEWLINE
Before the referenced commit, "#define foo#bar" would be accepted, (for
some definition of "accepted"[*]) while after the commit, the same input
causes the above error.
It's fairly obvious that that line is not what was intended by the
author of the shader, so a bug should be reported to the game
authors. Michel, can you follow up with that?
Meanwhile, I've looked at the GLSL specification and it's quite clear
that no '#' character is allowed except for introducing a directive, (so
at the beginning of a line, ignoring horizontal whitespace and
comments), or as part of a token-pasting operator in the replacement
list of a macro.
So I do think it's legitimate to generate a syntax error here.
But I also thought it would be worthwhile to look at what other
implementations do in a case like this. For example, given:
#define foo#bar
gcc's preprocessor emits a diagnostic:
warning: missing whitespace after the macro name [enabled by default]
and then goes on to behave as if what had been presented was:
#define foo #bar
(where "#bar" is literal text, and never interpreted as a directive).
For GLSL, we could do that, and if the foo macro is ever actually used,
the appearance of '#' should then generate a syntax error in the
compiler.
This is similar to "#define foo;bar" where gcc also generates the
warning and then defines "foo" to ";bar". For that case, glcpp is
already compatible with gcc, (though not generating the warning).
I also checked that Nvidia's implementation does accept the following
line without an error:
#define USE_DISCARD#line 0
It appears to be defining a macro USE_DISCARD to a value of "0",
(ignoring the "#line" portion). This isn't compatible with gcc, but
again, we're entirely outside the bounds of any specification with this
input.
I've experimented, and I've come up with a fairly simple patch that
would treat '#' in the same way that ';' is currently treated in a case
like this. That is:
#define USE_DISCARD#line 0
will define the USE_DISCARD macro to expand to "#line 0", (but not
treating #line as a directive).
Again, like I said above, I don't think that's strictly necessary
here. I believe the syntax error is legitimate, (and perhaps even
helpful---it pointed us to this bug in Reaction Quake's shader, for
example). But it is a possibility.
I'll follow up with that patch.
-Carl
[*] For reference, here's what previous versions of mesa would do with a
line like:
#define USE_DISCARD#line 0
1. Print a '#' character to stdout while parsing this shader
2. Define the USE_DISCARD macro to the text "line 0"
--
carl.d.wo...@intel.com
pgpo_d7KGLkWm.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
