[Mesa-dev] [RFC PATCH 06/12] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays

[Timothy Arceri]

On Mon, 2015-03-23 at 08:58 -0400, Ilia Mirkin wrote:
> On Sat, Mar 21, 2015 at 11:00 PM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> > On Sat, 2015-03-21 at 19:34 -0400, Ilia Mirkin wrote:
> >> On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> >> > Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
> >> > ---
> >> >  src/glsl/ast_to_hir.cpp | 6 +++++-
> >> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> >> > index 36f3eb7..7516e60 100644
> >> > --- a/src/glsl/ast_to_hir.cpp
> >> > +++ b/src/glsl/ast_to_hir.cpp
> >> > @@ -5595,7 +5595,11 @@ ast_interface_block::hir(exec_list *instructions,
> >> >                               _mesa_shader_stage_to_string(state->stage));
> >> >           }
> >> >           if (this->instance_name == NULL ||
> >> > -             strcmp(this->instance_name, "gl_in") != 0 || this->array_specifier == NULL) {
> >> > +             strcmp(this->instance_name, "gl_in") != 0 || this->array_specifier == NULL ||
> >> > +             (this->array_specifier->is_unsized_array &&
> >> > +              !this->array_specifier->array_dimensions.is_empty()) ||
> >>
> >> Don't know a lot about AST, but does it make sense to have
> >> ->is_unsized_array && array_dimensions? When would that be true?
> >> Shouldn't this just be !dim.empty() without the is_unsized_array
> >> check?
> >
> > ast is a pain to work with. I don't recall why I wrote it that way but
> > it should be easy to check as it was done so the piglit tests would
> > pass. I will take another look and see if I can rework it to be a little
> > cleaner. Otherwise I might just drop this patch for now.
> >
> 
> Please do, but I think that you do need the modification... just not
> the ->is_unsized_array bit of it. I think you want something like
> 
> || !is_empty || !is_tail
> 
> Basically you don't want gl_in[5], nor gl_in[5][] nor gl_in[][5] nor
> gl_in[][]. Right?

ok, so this is from the piglit test
glsl-1.50/compiler/gs-redeclares-pervertex-in-with-array-size.geom:

// Although the example mentions "gl_in[]", it seems reasonable to
// assume that specifying an array size (e.g. "gl_in[3]") is also
// allowed (this is confirmed by tests of the NVIDIA proprietary
// driver for Linux, version 313.18).

So we don't want gl_in, nor gl_in[5][] nor gl_in[][5] nor gl_in[][] but
gl_in[5] and gl_in[] are ok.

To answer your question from your first reply

> does it make sense to have ->is_unsized_array && array_dimensions?
When would that be true?

An unsized array doesn't have add anything to array_dimensions. So it
would be true for gl_in[][5].