That sounds good to me, I will submit a v3 with those edits. On Mon, Mar 28, 2016 at 9:19 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> 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 > > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
