[Mesa-dev] [PATCH] glsl: Reallow precision qualifiers on structure members

[Kenneth Graunke]

On 08/30/2013 02:35 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Changes to the grammar for GL_ARB_shading_language_420pack (commit
> 6eec502) moved precision qualifiers out of the type_specifier production
> chain.  This caused declarations such as:
>
>      struct S {
>          lowp float f;
>      };
>
> to generate parse errors.  Section 4.1.8 (Structures) of both the GLSL
> ES 1.00 spec and GLSL 1.30 specs says:
>
>          "Member declarators may contain precision qualifiers, but may not
>          contain any other qualifiers."
>
> So, it sure seems like we shouldn't generate a parse error. :)
>
> Instead of type_specifier, use fully_specified_type in struct members.
> However, fully_specified_type allows a lot of other qualifiers that are
> not allowed on structure members, so expeclitly disallow them.
>
> Note, this makes struct_declaration look an awful lot like
> member_declaration (used for interface blocks).  We may want to
> (somehow) unify these rules to reduce code duplication at some point.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68753
> Reported-by: Aras Pranckevicius <aras at unity3d.com>
> Cc: Aras Pranckevicius <aras at unity3d.com>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Matt Turner <mattst88 at gmail.com>
> Cc: "9.2" <mesa-stable at lists.freedesktop.org>
> ---
>   src/glsl/glsl_parser.yy | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index d8e589d..fa6e205 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1714,13 +1714,17 @@ struct_declaration_list:
>      ;
>
>   struct_declaration:
> -   type_specifier struct_declarator_list ';'
> +   fully_specified_type struct_declarator_list ';'
>      {
>         void *ctx = state;
> -      ast_fully_specified_type *type = new(ctx) ast_fully_specified_type();
> +      ast_fully_specified_type *const type = $1;
>         type->set_location(yylloc);
>
> -      type->specifier = $1;
> +      if (type->qualifier.flags.i != 0)
> +         _mesa_glsl_error(&@1, state,
> +			  "only precision qualifiers may be applied to "
> +			  "structure members");
> +
>         $$ = new(ctx) ast_declarator_list(type);
>         $$->set_location(yylloc);
>
>

Looks reasonable to me.  I thought this would let a few more qualifiers 
slip though, but it looks all of them except precision do have flags 
after all.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>