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

[Ian Romanick]

On 08/30/2013 03:34 PM, Kenneth Graunke wrote:
> 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.

I submitted a piglit patch that adds test cases for all the qualifiers
that I could find in the spec.  You might peek at that to see if there
are any others...

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