[Mesa-dev] [PATCH 7/8] glsl: Don't emit spurious errors for constant indexes of the wrong type

[Ian Romanick]

On 04/02/2013 10:58 AM, Kenneth Graunke wrote:
> On 04/01/2013 11:25 AM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Previously the shader
>>
>> uniform float x[6];
>> void main() { gl_Position.x = x[1.0]; }
>>
>> would have generated the errors
>>
>> 0:2(33): error: array index must be integer type
>> 0:2(36): error: array index must be < 6
>>
>> Now only
>>
>> 0:2(33): error: array index must be integer type
>>
>> will be generated.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>   src/glsl/ast_array_index.cpp | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
>> index 486ff55..c7ebcbd 100644
>> --- a/src/glsl/ast_array_index.cpp
>> +++ b/src/glsl/ast_array_index.cpp
>> @@ -58,7 +58,7 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>>       * declared size.
>>       */
>>      ir_constant *const const_index = idx->constant_expression_value();
>> -   if (const_index != NULL) {
>> +   if (const_index != NULL && idx->type->is_integer()) {
>>         const int idx = const_index->value.i[0];
>>         const char *type_name = "error";
>>         unsigned bound = 0;
>> @@ -118,7 +118,7 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>>           check_builtin_array_max_size(v->name, idx+1, loc, state);
>>        }
>>         }
>> -   } else if (array->type->is_array()) {
>> +   } else if (const_index == NULL && array->type->is_array()) {
>>         if (array->type->array_size() == 0) {
>>        _mesa_glsl_error(&loc, state, "unsized array index must be
>> constant");
>>         } else if (array->type->fields.array->is_interface()) {
>
> Aww.  Patch 6 cleaned this up so nicely, and now it's getting a bit
> uglier again.
>
> How about simply doing an early-return above:
>
>     if (!idx->type->is_integer()) {
>        _mesa_glsl_error(& idx_loc, state, "array index must be integer
> type");
>        return result;
>     } else if (!idx->type->is_scalar()) {
>        _mesa_glsl_error(& idx_loc, state, "array index must be scalar");
>        return result;
>     }
>
> Basically, if you hit those errors, you don't want to continue checking
> for more of them.

Hold that thought... we might want to come back to it.  One of the 
patches in the next series (watch for "glsl: Generate 
ir_binop_vector_extract for indexing of vectors") changes the way the 
value returned by this method is generated.  This makes the early-out 
paths annoying.  That's why patch 8 gets rid of them.