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

[Timothy Arceri]

On Thu, 2015-08-20 at 10:56 -0700, Ian Romanick wrote:
> On 08/20/2015 10:24 AM, Ian Romanick wrote:
> > On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> > > V2: move single dimensional array detection into a helper
> > > 
> > > Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
> > > Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > > ---
> > >  src/glsl/ast.h          | 8 ++++++++
> > >  src/glsl/ast_to_hir.cpp | 3 ++-
> > >  2 files changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> > > index d8c6cea..ab94cb2 100644
> > > --- a/src/glsl/ast.h
> > > +++ b/src/glsl/ast.h
> > > @@ -338,6 +338,14 @@ public:
> > >        array_dimensions.push_tail(&dim->link);
> > >     }
> > >  
> > > +   bool is_single_dimension()
> > 
> > I'd make this const... I believe all the list functions used here have
> > const versions.  With that change (or without it if it requires
> > additional list overloads), this patch is
> > 
> > Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
> > 
> > > +   {
> > > +      return (this->is_unsized_array && this
> > > ->array_dimensions.is_empty()) ||
> > > +         (!this->is_unsized_array &&
> > > +          this->array_dimensions.tail_pred->prev != NULL &&
> > > +          this->array_dimensions.tail_pred->prev->is_head_sentinel());
> 
> After looking at patch 9, I think I have a suggestion for this.  The
> array is one dimension if this->array_dimensions has exactly one
> element, correct?  In that case, I think this is more clear:
> 
>     /* If the list is not empty and the first element is the only
>      * element, the array is one-dimensional.
>      */
>     return this->array_dimensions->get_head() != NULL &&
>            this->array_dimensions->get_head()->get_next()
> ->is_tail_sentinel());

Right patch 9 fixes the wrong assumptions I originally made (almost 2 years
ago) that only the outermost arrays can be unsized, I'll move this patch to
after patch 9 and implement your suggestion. Thanks.


> 
> > > +   }
> > > +
> > >     virtual void print(void) const;
> > >  
> > >     /* If true, this means that the array has an unsized outermost 
> > > dimension. */
> > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> > > index 06cd6a5..5ee02ed 100644
> > > --- a/src/glsl/ast_to_hir.cpp
> > > +++ b/src/glsl/ast_to_hir.cpp
> > > @@ -5962,7 +5962,8 @@ 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_single_dimension()) {
> > >              _mesa_glsl_error(&loc, state,
> > >                               "gl_PerVertex input must be redeclared as 
> > > "
> > >                               "gl_in[]");
> > > 
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
>