[Mesa-dev] [PATCH 07/11] glsl: error out on empty declarations

[Iago Toral]

On Mon, 2015-01-19 at 19:03 -0800, Ian Romanick wrote:
> On 01/19/2015 04:52 AM, Erik Faye-Lund wrote:
> > On Mon, Jan 19, 2015 at 12:32 PM, Eduardo Lima Mitev <elima at igalia.com> wrote:
> >> From: Iago Toral Quiroga <itoral at igalia.com>
> >>
> >> So far we have only been emitting a warning.
> >>
> >> Fixes the following 2 dEQP tests:
> >> dEQP-GLES3.functional.shaders.arrays.invalid.empty_declaration_without_var_name_vertex
> >> dEQP-GLES3.functional.shaders.arrays.invalid.empty_declaration_without_var_name_fragment
> >> ---
> >>  src/glsl/ast_to_hir.cpp | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> >> index c52e4af..5ed8b0e 100644
> >> --- a/src/glsl/ast_to_hir.cpp
> >> +++ b/src/glsl/ast_to_hir.cpp
> >> @@ -3406,7 +3406,7 @@ ast_declarator_list::hir(exec_list *instructions,
> >>                 "lowp"
> >>              };
> >>
> >> -            _mesa_glsl_warning(&loc, state,
> >> +            _mesa_glsl_error(&loc, state,
> >>                                 "empty declaration with precision qualifier, "
> >>                                 "to set the default precision, use "
> >>                                 "`precision %s %s;'",
> >> @@ -3414,7 +3414,7 @@ ast_declarator_list::hir(exec_list *instructions,
> >>                                 type_name);
> >>           }
> >>        } else if (this->type->specifier->structure == NULL) {
> >> -         _mesa_glsl_warning(&loc, state, "empty declaration");
> >> +         _mesa_glsl_error(&loc, state, "empty declaration");
> >>        }
> >>     }
> >>
> > 
> > OpenGL ES 3.1 specifies this as allowed, as far as I can tell:
> > 
> > Section 4.11:
> > "Empty declarations are allowed. E.g.
> > 
> > int; // No effect
> > struct S {int x;}; // Defines a struct S"
> > 
> > OpenGL ES 3.0 seems to not say, which generally should be interpreted
> > as "allowed". In fact, no such error is listed in section 10 of the
> > spec, which is the list of errors required to detect.

We included some text in the cover letter of this series about this
topic. I think that GL ES3 explicitly allows it but you have to go to
the chapter "Shading Language Grammar" (page 106) to see it. There they
have this:

declaration:
   <various rules here>
   type_qualifier SEMICOLON

> > So I think these dEQP-tests are bogus.
> 
> I think Erik is right.  I seem to recall submitting a Khronos bug about
> this (long, long ago), and the decision was that this was silly but valid.

Agreed, let's drop this patch.

Iago

> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev