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

[Kenneth Graunke]

On Monday, March 7, 2016 5:23:26 PM PST Ian Romanick wrote:
> 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>

Good call.  I verified that Unigine Heaven's shaders fail to compile
normally, but work with allow_glsl_extension_directive_midshader=true,
even after my patch series.  The app itself also continues running fine.

Thanks for the review!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160307/8d783003/attachment.sig>