[Mesa-dev] [PATCH] glsl/linker: check same name is not used in block and outside

[Juan A. Suarez Romero]

On Wed, 2018-01-31 at 01:38 +0100, Matteo Bruni wrote:
> 2018-01-30 16:11 GMT+01:00 Juan A. Suarez Romero <jasuarez at igalia.com>:
> > According with OpenGL GLSL 3.20 spec, section 4.3.9:
> > 
> >   "It is a link-time error if any particular shader interface
> >    contains:
> >      - two different blocks, each having no instance name, and each
> >        having a member of the same name, or
> >      - a variable outside a block, and a block with no instance name,
> >        where the variable has the same name as a member in the block."
> > 
> > This fixes a previous commit 9b894c8 ("glsl/linker: link-error using the
> > same name in unnamed block and outside") that covered this case, but
> > did not take in account that precision qualifiers are ignored when
> > comparing blocks with no instance name.
> 
> Thanks! This also fixes Wine, although the reason is slightly
> different: Wine might end up linking a program with a vertex shader
> containing "#extension GL_ARB_cull_distance : enable" together with a
> vertex shader without it. I guess this makes the gl_PerVertex builtin
> defined in the two shader objects differ and that triggered the old
> check but not the new one from this patch.
> 
> I don't know if you want to change the commit message to account for
> that (maybe make it more general?) but in any case, and FWIW:
> 
> Tested-by: Matteo Bruni <matteo.mystral at gmail.com>
> 
> I also have a nitpick which you can entirely ignore, below.
> 
> > +         if (var->get_interface_type() != existing->get_interface_type()) {
> > +            if (!var->get_interface_type() || !existing->get_interface_type()) {
> > +               linker_error(prog, "declarations for %s `%s` are inside block "
> > +                            "`%s` and outside a block",
> > +                            mode_string(var), var->name,
> > +                            var->get_interface_type()
> > +                               ? var->get_interface_type()->name
> > +                               : existing->get_interface_type()->name);
> > +               return;
> > +            } else if (strcmp(var->get_interface_type()->name,
> > +                              existing->get_interface_type()->name) != 0) {
> > +               linker_error(prog, "declarations for %s `%s` are inside blocks "
> > +                            "`%s` and `%s`",
> > +                            mode_string(var), var->name,
> > +                            existing->get_interface_type()->name,
> > +                            var->get_interface_type()->name);
> 
> What about declaring a couple of variables for the interface types and
> using them through? Like:
> 
> const glsl_type var_itype = var->get_interface_type();
> const glsl_type existing_itype = existing->get_interface_type();
> 

Yes, I'll use a couple of variables. Thanks.


	J.A.

> (maybe with different names, I don't know what's Mesa's preferred style)
>