[Mesa-dev] [PATCH] glsl parser: Select typelist as early as possible.

[Eric Anholt]

On Wed, 11 Apr 2012 10:56:51 +0200, Olivier Galibert <galibert at pobox.com> wrote:
> Type selection must be extended w.r.t version and extension lines as
> soon as it is possible, and in any case before the lookahead is done
> to check the nature of the next line.  Otherwise code such as:
> 
>   #version 400
> 
>   dmat2 function inverse(dmat2 m) { ... }
> 
> fails because "dmat2" has already been lexed as a NEW_IDENTIFIER
> before _mesa_glsl_initialize_types is called.
> 
> Signed-off-by: Olivier Galibert <galibert at pobox.com>
>
> ---
>  src/glsl/glsl_parser.yy |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> [Sorry, resent with the correct subject...]
> 
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 64506b6..9a0af95 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -233,9 +233,6 @@ static void yyerror(YYLTYPE *loc, _mesa_glsl_parse_state *st, const char *msg)
>  
>  translation_unit: 
>  	version_statement extension_statement_list
> -	{
> -	   _mesa_glsl_initialize_types(state);
> -	}
>  	external_declaration_list
>  	{
>  	   delete state->symbols;
> @@ -285,6 +282,7 @@ version_statement:
>  			       state->version_string,
>  			       state->supported_version_string);
>  	   }
> +	   _mesa_glsl_initialize_types(state);
>  	}
>  	;
>  
> @@ -322,6 +320,7 @@ extension_statement:
>  	   if (!_mesa_glsl_process_extension($2, & @2, $4, & @4, state)) {
>  	      YYERROR;
>  	   }
> +	   _mesa_glsl_initialize_types(state);
>  	}

What I was concerned about here was that we would add the same type
multiple times when a #extension is present, and then all hell would
break loose.  It looks like actually the type is generated and then not
added to the symbol table when it already exists there, so this looks
fine to me.

Testing with:

#version 120
mat2x4 a;

I guess that because all previous new types have been turned into
keywords, and our handling of keywords at parse time ensures that they
don't get treated as NEW_IDENTIFIER, this hasn't been a problem before.
dmat2 is also a keyword.  So I'm not actually sure if this patch is
really worth it (initializing the types in the symbol table twice, when
you shouldn't ever be able to hit the bug it's fixing).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120413/ff00d41d/attachment.pgp>