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

Emil Velikov emil.l.velikov at gmail.com
Sun Nov 8 04:07:10 PST 2015


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.

<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


More information about the mesa-dev mailing list