On 08/30/2013 03:34 PM, Kenneth Graunke wrote: > On 08/30/2013 02:35 PM, Ian Romanick wrote: >> From: Ian Romanick <[email protected]> >> >> 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 <[email protected]> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68753 >> Reported-by: Aras Pranckevicius <[email protected]> >> Cc: Aras Pranckevicius <[email protected]> >> Cc: Kenneth Graunke <[email protected]> >> Cc: Matt Turner <[email protected]> >> Cc: "9.2" <[email protected]> >> --- >> 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 <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
