On Monday, March 28, 2016 8:16:17 PM PDT Lars Hamre wrote: > NOTE: someone with access will need to commit this patch after the > review process > > Invalidates float suffixes for glsl 1.10 > > Fixes the following piglit tests: > tests/spec/glsl-1.10/compiler/literals/invalid-float-suffix-capital-f.vert > tests/spec/glsl-1.10/compiler/literals/invalid-float-suffix-f.vert` > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81585 > > Signed-off-by: Lars Hamre <cheme...@gmail.com> > Reviewed-by: Timothy Arceri <timothy.arc...@collabora.com> > > --- > src/compiler/glsl/glsl_lexer.ll | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/src/compiler/glsl/glsl_lexer.ll b/src/compiler/glsl/ glsl_lexer.ll > index 1f12265..0980f4f 100644 > --- a/src/compiler/glsl/glsl_lexer.ll > +++ b/src/compiler/glsl/glsl_lexer.ll > @@ -472,8 +472,17 @@ layout { > \.[0-9]+([eE][+-]?[0-9]+)?[fF]? | > [0-9]+\.([eE][+-]?[0-9]+)?[fF]? | > [0-9]+[eE][+-]?[0-9]+[fF]? { > - yylval->real = _mesa_strtof(yytext, NULL); > - return FLOATCONSTANT; > + struct _mesa_glsl_parse_state *state = yyextra; > + char suffix = yytext[strlen(yytext) - 1]; > + if (!state->is_version(120, 0) && > + (suffix == 'f' || suffix == 'F')) { > + _mesa_glsl_error(yylloc, state, > + "Float suffixes are invalid in > GLSL 1.10"); > + return ERROR_TOK; > + } else { > + yylval->real = _mesa_strtof(yytext, NULL); > + return FLOATCONSTANT; > + }
Hi Lars,
Good catch! Would it also work to do:
if (!state->is_version(120, 0) &&
(suffix == 'f' || suffix == 'F')) {
_mesa_glsl_error(yylloc, state,
"Float suffixes are invalid in GLSL
1.10");
}
yylval->real = _mesa_strtof(yytext, NULL);
return FLOATCONSTANT;
In other words, raise the error and fail the compile, but parse the
float literal as intended. I think returning ERROR_TOK is likely to
make the parser hit bison-generated "Expected <SOME TOKENS>, got <SOME
OTHER TOKENS>" errors that are pretty cryptic. Parsing the number in
the obvious manner would let the compile proceed normally, so we don't
get cascading errors that might be confusing.
That patch would get a:
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
> }
>
> [0-9]+\.[0-9]+([eE][+-]?[0-9]+)?(lf|LF) |
> --
> 2.5.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
