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.
Reviewed-by: Kenneth Graunke <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
