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

[Matteo Bruni]

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();

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