Re: [Mesa-dev] [PATCH v2] compiler/glsl: invalidate float suffixes for glsl 1.10
That sounds good to me, I will submit a v3 with those edits. On Mon, Mar 28, 2016 at 9:19 PM, Kenneth Graunke 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 > > Reviewed-by: Timothy Arceri > > > > --- > > 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 , got 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 > > > } > > > > [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
Re: [Mesa-dev] [PATCH v2] compiler/glsl: invalidate float suffixes for glsl 1.10
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 > Reviewed-by: Timothy Arceri > > --- > 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 , got " 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 > } > > [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
[Mesa-dev] [PATCH v2] compiler/glsl: invalidate float suffixes for glsl 1.10
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 Reviewed-by: Timothy Arceri --- 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; + } } [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