Re: [Mesa-dev] [PATCH v2] compiler/glsl: invalidate float suffixes for glsl 1.10

2016-03-28 Thread Lars Hamre
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

2016-03-28 Thread Kenneth Graunke
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

2016-03-28 Thread Lars Hamre
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