On 25/02/16 10:41, Timothy Arceri wrote: > On Thu, 2016-02-25 at 09:09 +0100, Alejandro Piñeiro wrote: >> 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. > But isn't the lhs always just a variable so you would just set the flag > to true on the lhs subexpression for an assignment. The lhs is never > actually an expression or I'm I missing something?
Well, when is_lhs is assigned, it is an expression, but you are right, I just checked and it is with just the one variable we want. I thought that there were more "expression layers" between the node with this->oper==ast_identifier, and the one that does the assignment. Thanks for the advices, I think that I have everything I need now. > >> 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
