[Mesa-dev] [PATCH] glsl: do not allow interface block to have name already taken

[Tapani]

On 01/20/2015 08:29 PM, Ian Romanick wrote:
> On 01/19/2015 10:55 PM, Tapani Pälli wrote:
>> Fixes currently failing Piglit case
>>     interface-blocks-name-reused-globally.vert
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/glsl/ast_to_hir.cpp | 17 ++++++++++++++++-
>>   1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 811a955..13ddb00 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -5443,9 +5443,24 @@ ast_interface_block::hir(exec_list *instructions,
>>   
>>      state->struct_specifier_depth--;
>>   
>> -   if (!redeclaring_per_vertex)
>> +   if (!redeclaring_per_vertex) {
>> +      ir_variable *var;
> This is C++, so you can combine the declaration with the assignment
> below.  I guess you've been bit by the C89 requirements elsewhere in
> Mesa. :)

True :) Thanks, I'll send a v2

>>         validate_identifier(this->block_name, loc, state);
>>   
>> +      /* From section 4.3.9 ("Interface Blocks") of the GLSL 4.50 spec:
>> +       *
>> +       *     "Block names have no other use within a shader beyond interface
>> +       *     matching; it is a compile-time error to use a block name at global
>> +       *     scope for anything other than as a block name."
>> +       */
>> +      var = state->symbols->get_variable(this->block_name);
>> +      if (var && !var->type->is_interface()) {
>> +         _mesa_glsl_error(&loc, state, "Block name `%s' is "
>> +                          "already used in the scope.",
>> +                          this->block_name);
>> +      }
> This fixes the previously mentioned test case, but what about a test like
>
> out block {
>      vec4 a;
> } inst;
>
> vec4 block;
>
> Looking at Chris's patch (piglit 14165586) that adds
> interface-blocks-name-reused-globally.vert, I don't see a test like
> this.  Does that test already exist, and I just missed it?
>
>> +   }
>> +
>>      const glsl_type *earlier_per_vertex = NULL;
>>      if (redeclaring_per_vertex) {
>>         /* Find the previous declaration of gl_PerVertex.  If we're redeclaring
>>