[Mesa-dev] [RFC 3/3] glsl: raise warning when using uninitialized variables

[Alejandro Piñeiro]

On 25/02/16 00:27, Timothy Arceri wrote:
> On Wed, 2016-02-24 at 20:04 +0100, Alejandro Piñeiro wrote:
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94129
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index ee5485c..296c845 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -1885,6 +1885,13 @@ ast_expression::do_hir(exec_list
>> *instructions,
>>        if (var != NULL) {
>>           var->data.used = true;
>>           result = new(ctx) ir_dereference_variable(var);
>> +
>> +         if (var->data.mode == ir_var_auto
> I think we would also maybe want to warn for output varyings???
>
> var->data.mode == ir_var_shader_out

Ok, I will check this one.


>> +             && !assignment_recipient
>> +             && result->variable_referenced()->data.assigned !=
>> true) {
>> +            _mesa_glsl_warning(& loc, state, "`%s' used 
>                                 ---^
> We don't really use this style anymore   

Well, I just used the style of the mesa_glsl_error three lines below :/.
Thanks for pointing it, I would not have realized it, I will update the
code.

>> uninitialized",
>> +                               this->primary_expression.identifier);
>> +         }
> I think what you have will work but I don't really like passing
> around assignment_recipient. Maybe it would be better just to add a new
> bool lhs_var; to the ast_expression class that defaults to false and
> use that instead.

At some point I thought on that idea too, but as I mentioned on the
cover letter, on the ast tree we don't have access to the parent
expression, that is the one that would get lhs_var assigned. The only
solution to that problem that I found is that when being assigned to a
expression, it would need to go for all the children and assigning that
value, something that was also somewhat intrusive and probably slower
compared to what I finally implemented. In any case, I will try to
implement that variant, so reviewers would decide what is the best option.

Thanks for all the feedback.

BR