[Mesa-dev] [PATCH 3/6] glsl: move layout qualifier validation out of the parser

[Timothy Arceri]

On Sun, 2015-11-08 at 12:07 +0000, Emil Velikov wrote:
> On 6 November 2015 at 21:13, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> > On Fri, 2015-11-06 at 13:16 +0000, Emil Velikov wrote:
> > > On 5 November 2015 at 11:17, Timothy Arceri <t_arceri at yahoo.com.au>
> > > wrote:
> > > > From: Timothy Arceri <timothy.arceri at collabora.com>
> > > > 
> > > > This is in preperation for compile-time constant support.
> > > typo "preparation"
> > > 
> > > > 
> > > > Also fix up the locations for some of the extension checking
> > > > error messages in the parser. We now correctly give the location
> > > > of the layout qualifier identifier rather than the integer constant.
> > > > 
> > > > The validation is moved to two locations, for validation on variables
> > > > the
> > > > checks are moved to the ast to hir pass and for qualifiers that apply
> > > > to
> > > > the
> > > > shader the validation is moved into glsl_parser_extras.cpp.
> > > > 
> > > > In order to do validation at the later stage in glsl_parser_extras.cpp
> > > > we
> > > > need to temporarily add a field in ast_type_qualifier to keep track of
> > > > the
> > > > parser location, this will be removed in a following patch when we
> > > > introduce a new type for storing the comiple-time qualifiers.
> > > > 
> > > > Also as the set_shader_inout_layout() function in glsl parser extras
> > > > is
> > > > normally called after all validation is done we need to move the code
> > > > that
> > > > sets CompileStatus and InfoLog otherwise the newly moved error
> > > > messages
> > > > will
> > > > be ignored.
> > > Personally I would split the validate_layout_qualifiers() introduction
> > > and the CompileStatus/InfoLog movement into separate patches.
> > 
> > The reason for not doing this in a new patch is that this is existing
> > functionality not new functionality, doing so would regress a bunch of
> > piglit
> > tests.
> > 
> > I can do it if it makes things easier to review but it should all be
> > pushed as
> > one.
> > 
> Fair enough - I'd just keep in as it then. I'll take a closer look at
> some time today/tomorrow.

On second thoughts I should be able to break this up into shader level layouts
and per variable layouts I think I missunderstood what you were getting at
when reading your first reply.

I have a version 2 in progress based on your other feedback so you might want
to hold of reviewing until I send that. 

> 
> <rant>
> Imho if one needs to made a few different things at once, this is a
> clear indication that things are more convoluted as they should be.
> </rant>
> 
> Cheers,
> Emil