<div dir="ltr">That sounds good to me, I will submit a v3 with those edits. </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 28, 2016 at 9:19 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=81585" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=81585</a> > > Signed-off-by: Lars Hamre <<a href="mailto:chemecse@gmail.com">chemecse@gmail.com</a>> > Reviewed-by: Timothy Arceri <<a href="mailto:timothy.arceri@collabora.com">timothy.arceri@collabora.com</a>> > > --- > 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; > + } </div></div>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 <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> > } > > [0-9]+\.[0-9]+([eE][+-]?[0-9]+)?(lf|LF) | > -- > 2.5.5 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > </blockquote></div> </div>