[Mesa-dev] [PATCH 08/17] glsl parser: allow in & out for interface block members

[Jordan Justen]

On Sun, Apr 21, 2013 at 12:06 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On 04/19/2013 12:35 PM, Jordan Justen wrote:
>>
>> Previously uniform blocks allowed for the 'uniform' keyword
>> to be used with members of a uniform blocks. With interface
>> blocks 'in' can be used on 'in' interface block members and
>> 'out' can be used on 'out' interface block members.
>>
>> The basic_interface_block rule will verify that the same
>> qualifier type is used with the block and each member.
>>
>> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>> ---
>>   src/glsl/glsl_parser.yy |   47
>> ++++++++++++++++++++++++++++++-----------------
>>   1 file changed, 30 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
>> index c254a2f..6d88263 100644
>> --- a/src/glsl/glsl_parser.yy
>> +++ b/src/glsl/glsl_parser.yy
>> @@ -2051,41 +2051,54 @@ member_list:
>>         }
>>         ;
>>
>> -/* Specifying "uniform" inside of a uniform block is redundant. */
>> -uniformopt:
>> -       /* nothing */
>> -       | UNIFORM
>> -       ;
>> -
>>   member_declaration:
>> -       layout_qualifier uniformopt type_specifier struct_declarator_list
>> ';'
>> +       layout_qualifier fully_specified_type struct_declarator_list ';'
>
>
> I don't think this is right.  The fully_specified_type production rule
> expands to "type_qualifier type_specifier".  type_qualifier expands to:
>
> type_qualifier:
>         storage_qualifier
>         | layout_qualifier
>         | layout_qualifier storage_qualifier
>
> at which point you have:
>
> layout_qualifier layout_qualifier storage_qualifier type_specifier
> struct_declarator_list ';'
>
> I like the move to use fully_specified_type, but I think you need to make
> the rule simply:
>
>         fully_specified_type struct_declarator_list ';'

This seems to work, so I'll make this change.

> I'm also a bit concerned that this may allow too much.  For example, is
> "invariant" allowed?  If not, are you checking that somewhere?

The 1.50 spec says (of interface block qualifiers) that: "If optional
qualifiers are used, they can include interpolation and storage
qualifiers" ...

I think this (vaguely) means that invariant, highp, mediump, and lowp
should not be allowed.

Yet, nVidia seems to allow them. My opinion is that given the vague
spec language, we should allow them. What's your opinion?

-Jordan