[Mesa-dev] [PATCH 05/13] glsl: simplifies the merge of the default in layout qualifier

[Timothy Arceri]

On Tue, 2016-11-22 at 16:07 +0200, Andres Gomez wrote:
> On Tue, 2016-11-22 at 22:22 +1100, Timothy Arceri wrote:
> ...
> > 
> > 
> > Can I ask that you write a follow up patch for this series that
> > creates
> > a helper function for each of these validations. For example:
> > 
> > static bool
> > validate_ordering(loc, state, qualifier, new_qualifier)
> > {
> >    if (qualifier->flags.q.ordering && new_qualifier-
> > >flags.q.ordering
> >        && qualifier->ordering != new_qualifier->ordering) {
> >       _mesa_glsl_error(loc, state,
> >                        "conflicting ordering specified");
> >    }
> > 
> >    return true;
> > }
> > 
> > In merge_qualifier() just put the call in the outer if. For
> > example:
> > 
> > if (q.flags.q.ordering && validate_ordering(...))
> > 
> > This will be better IMO as the code currently exists early when
> > there
> > is no reason we shouldn't continue on and check the other layout
> > qualifiers before reporting errors.
> 
> OK, I will continue with this follow up but I don't think I can just
> put the check in the outer if since, then, I will modify the
> returning
> value of the function in some cases. I will work on that.

_mesa_glsl_error() will set state->error to true so you should be able
to use that at the end of merge_qualifier()

> 
> I also think that continuing without exiting may cause an unexpected
> problem but I will do a piglit and cts run to check.
> 

It should be fine but piglit will tell us for sure :)