[Mesa-dev] [PATCH] glsl: Remove unreachable error message

[Ian Romanick]

On 12/04/2013 10:32 PM, Timothy Arceri wrote:
> On Wed, 2013-12-04 at 14:32 -0800, Ian Romanick wrote:
>> On 11/20/2013 03:41 AM, Timothy Arceri wrote:
>>> Left over from bug #34376.
>>
>> I think this shader hits this error message:
>>
>>     #version 120
>>
>>     int x[];
>>
>>     void foo() { x[3] = 2; }
>>
>>     int x[] = int[2](1,2);
>>
>> do_assignment is also used for initializers.  Initializers can be used
>> to (explicitly) size arrays, and arrays declared without an explicit
>> size can be redeclared with an explicit size.
> 
> Thanks Ian, mark this down as a newbie mistake I failed to recognise
> this type of scenario would cause max_array_access to be set, although
> its now obvious when I think about it. 

Well... it took me a few minutes to come up with a test case that could
hit that path.  When I saw the patch, I felt confident that I had added
that error check for a reason, but I couldn't remember what it was.

> I guess this is a good candidate for a piglit test to help spot

Yes.  I'll submit it later today.

> regressions with the ARB_arrays_of_arrays work. I've already submitted
> one test for the same reason [1] would it be better to save a bunch of
> these tests up and submit them as a series or is it fine to submit them
> one by one?

If they're tests, like the one below, that can run now, you can submit
them however is most convenient for you.

> [1]
> http://lists.freedesktop.org/archives/piglit/2013-December/008657.html 
>>
>>> Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
>>> ---
>>>  src/glsl/ast_to_hir.cpp | 7 -------
>>>  1 file changed, 7 deletions(-)
>>>
>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>> index 76b256c..adf10a0 100644
>>> --- a/src/glsl/ast_to_hir.cpp
>>> +++ b/src/glsl/ast_to_hir.cpp
>>> @@ -813,13 +813,6 @@ do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state *state,
>>>  
>>>  	 assert(var != NULL);
>>>  
>>> -	 if (var->max_array_access >= unsigned(rhs->type->array_size())) {
>>> -	    /* FINISHME: This should actually log the location of the RHS. */
>>> -	    _mesa_glsl_error(& lhs_loc, state, "array size must be > %u due to "
>>> -			     "previous access",
>>> -			     var->max_array_access);
>>> -	 }
>>> -
>>>  	 var->type = glsl_type::get_array_instance(lhs->type->element_type(),
>>>  						   rhs->type->array_size());
>>>  	 d->type = var->type;
>>>
>>
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev