glsl: add is_lhs bool on ast_expression
Useful to know if a expression is the recipient of an assignment or not, that would be used to (for example) raise warnings of "use of uninitialized variable" without getting a false positive when assigning first a variable. By default the value is false, and it is assigned to true on the following cases: * The lhs assignments subexpression * At ast_array_index, on the array itself. * While handling the method on an array, to avoid the warning calling array.length * When computed the cached test expression at test_to_hir, to avoid a duplicate warning on the test expression of a switch. set_is_lhs setter is added, because in some cases (like ast_field_selection) the value need to be propagated on the expression tree. To avoid doing the propatagion if not needed, it skips if no primary_expression.identifier is available. v2: use a new bool on ast_expression, instead of a new parameter on ast_expression::hir (Timothy Arceri) v3: fix style and some typos on comments, initialize is_lhs default value on constructor, to avoid a c++11 feature (Ian Romanick) v4: some tweaks on comments (Timothy Arceri) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94129 Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
This commit is contained in:
@@ -214,6 +214,7 @@ public:
|
|||||||
subexpressions[2] = NULL;
|
subexpressions[2] = NULL;
|
||||||
primary_expression.identifier = identifier;
|
primary_expression.identifier = identifier;
|
||||||
this->non_lvalue_description = NULL;
|
this->non_lvalue_description = NULL;
|
||||||
|
this->is_lhs = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
static const char *operator_string(enum ast_operators op);
|
static const char *operator_string(enum ast_operators op);
|
||||||
@@ -263,6 +264,11 @@ public:
|
|||||||
* This pointer may be \c NULL.
|
* This pointer may be \c NULL.
|
||||||
*/
|
*/
|
||||||
const char *non_lvalue_description;
|
const char *non_lvalue_description;
|
||||||
|
|
||||||
|
void set_is_lhs(bool new_value);
|
||||||
|
|
||||||
|
private:
|
||||||
|
bool is_lhs;
|
||||||
};
|
};
|
||||||
|
|
||||||
class ast_expression_bin : public ast_expression {
|
class ast_expression_bin : public ast_expression {
|
||||||
|
@@ -1727,6 +1727,10 @@ ast_function_expression::handle_method(exec_list *instructions,
|
|||||||
const char *method;
|
const char *method;
|
||||||
method = field->primary_expression.identifier;
|
method = field->primary_expression.identifier;
|
||||||
|
|
||||||
|
/* This would prevent to raise "uninitialized variable" warnings when
|
||||||
|
* calling array.length.
|
||||||
|
*/
|
||||||
|
field->subexpressions[0]->set_is_lhs(true);
|
||||||
op = field->subexpressions[0]->hir(instructions, state);
|
op = field->subexpressions[0]->hir(instructions, state);
|
||||||
if (strcmp(method, "length") == 0) {
|
if (strcmp(method, "length") == 0) {
|
||||||
if (!this->expressions.is_empty()) {
|
if (!this->expressions.is_empty()) {
|
||||||
|
@@ -1248,6 +1248,24 @@ ast_expression::hir_no_rvalue(exec_list *instructions,
|
|||||||
do_hir(instructions, state, false);
|
do_hir(instructions, state, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void
|
||||||
|
ast_expression::set_is_lhs(bool new_value)
|
||||||
|
{
|
||||||
|
/* is_lhs is tracked only to print "variable used uninitialized" warnings,
|
||||||
|
* if we lack a identifier we can just skip it.
|
||||||
|
*/
|
||||||
|
if (this->primary_expression.identifier == NULL)
|
||||||
|
return;
|
||||||
|
|
||||||
|
this->is_lhs = new_value;
|
||||||
|
|
||||||
|
/* We need to go through the subexpressions tree to cover cases like
|
||||||
|
* ast_field_selection
|
||||||
|
*/
|
||||||
|
if (this->subexpressions[0] != NULL)
|
||||||
|
this->subexpressions[0]->set_is_lhs(new_value);
|
||||||
|
}
|
||||||
|
|
||||||
ir_rvalue *
|
ir_rvalue *
|
||||||
ast_expression::do_hir(exec_list *instructions,
|
ast_expression::do_hir(exec_list *instructions,
|
||||||
struct _mesa_glsl_parse_state *state,
|
struct _mesa_glsl_parse_state *state,
|
||||||
@@ -1323,6 +1341,7 @@ ast_expression::do_hir(exec_list *instructions,
|
|||||||
break;
|
break;
|
||||||
|
|
||||||
case ast_assign: {
|
case ast_assign: {
|
||||||
|
this->subexpressions[0]->set_is_lhs(true);
|
||||||
op[0] = this->subexpressions[0]->hir(instructions, state);
|
op[0] = this->subexpressions[0]->hir(instructions, state);
|
||||||
op[1] = this->subexpressions[1]->hir(instructions, state);
|
op[1] = this->subexpressions[1]->hir(instructions, state);
|
||||||
|
|
||||||
@@ -1592,6 +1611,7 @@ ast_expression::do_hir(exec_list *instructions,
|
|||||||
case ast_div_assign:
|
case ast_div_assign:
|
||||||
case ast_add_assign:
|
case ast_add_assign:
|
||||||
case ast_sub_assign: {
|
case ast_sub_assign: {
|
||||||
|
this->subexpressions[0]->set_is_lhs(true);
|
||||||
op[0] = this->subexpressions[0]->hir(instructions, state);
|
op[0] = this->subexpressions[0]->hir(instructions, state);
|
||||||
op[1] = this->subexpressions[1]->hir(instructions, state);
|
op[1] = this->subexpressions[1]->hir(instructions, state);
|
||||||
|
|
||||||
@@ -1618,6 +1638,7 @@ ast_expression::do_hir(exec_list *instructions,
|
|||||||
}
|
}
|
||||||
|
|
||||||
case ast_mod_assign: {
|
case ast_mod_assign: {
|
||||||
|
this->subexpressions[0]->set_is_lhs(true);
|
||||||
op[0] = this->subexpressions[0]->hir(instructions, state);
|
op[0] = this->subexpressions[0]->hir(instructions, state);
|
||||||
op[1] = this->subexpressions[1]->hir(instructions, state);
|
op[1] = this->subexpressions[1]->hir(instructions, state);
|
||||||
|
|
||||||
@@ -1640,6 +1661,7 @@ ast_expression::do_hir(exec_list *instructions,
|
|||||||
|
|
||||||
case ast_ls_assign:
|
case ast_ls_assign:
|
||||||
case ast_rs_assign: {
|
case ast_rs_assign: {
|
||||||
|
this->subexpressions[0]->set_is_lhs(true);
|
||||||
op[0] = this->subexpressions[0]->hir(instructions, state);
|
op[0] = this->subexpressions[0]->hir(instructions, state);
|
||||||
op[1] = this->subexpressions[1]->hir(instructions, state);
|
op[1] = this->subexpressions[1]->hir(instructions, state);
|
||||||
type = shift_result_type(op[0]->type, op[1]->type, this->oper, state,
|
type = shift_result_type(op[0]->type, op[1]->type, this->oper, state,
|
||||||
@@ -1658,6 +1680,7 @@ ast_expression::do_hir(exec_list *instructions,
|
|||||||
case ast_and_assign:
|
case ast_and_assign:
|
||||||
case ast_xor_assign:
|
case ast_xor_assign:
|
||||||
case ast_or_assign: {
|
case ast_or_assign: {
|
||||||
|
this->subexpressions[0]->set_is_lhs(true);
|
||||||
op[0] = this->subexpressions[0]->hir(instructions, state);
|
op[0] = this->subexpressions[0]->hir(instructions, state);
|
||||||
op[1] = this->subexpressions[1]->hir(instructions, state);
|
op[1] = this->subexpressions[1]->hir(instructions, state);
|
||||||
type = bit_logic_result_type(op[0], op[1], this->oper, state, &loc);
|
type = bit_logic_result_type(op[0], op[1], this->oper, state, &loc);
|
||||||
@@ -1839,6 +1862,11 @@ ast_expression::do_hir(exec_list *instructions,
|
|||||||
case ast_array_index: {
|
case ast_array_index: {
|
||||||
YYLTYPE index_loc = subexpressions[1]->get_location();
|
YYLTYPE index_loc = subexpressions[1]->get_location();
|
||||||
|
|
||||||
|
/* Getting if an array is being used uninitialized is beyond what we get
|
||||||
|
* from ir_value.data.assigned. Setting is_lhs as true would force to
|
||||||
|
* not raise a uninitialized warning when using an array
|
||||||
|
*/
|
||||||
|
subexpressions[0]->set_is_lhs(true);
|
||||||
op[0] = subexpressions[0]->hir(instructions, state);
|
op[0] = subexpressions[0]->hir(instructions, state);
|
||||||
op[1] = subexpressions[1]->hir(instructions, state);
|
op[1] = subexpressions[1]->hir(instructions, state);
|
||||||
|
|
||||||
@@ -5746,6 +5774,11 @@ ast_switch_statement::test_to_hir(exec_list *instructions,
|
|||||||
{
|
{
|
||||||
void *ctx = state;
|
void *ctx = state;
|
||||||
|
|
||||||
|
/* set to true to avoid a duplicate "use of uninitialized variable" warning
|
||||||
|
* on the switch test case. The first one would be already raised when
|
||||||
|
* getting the test_expression at ast_switch_statement::hir
|
||||||
|
*/
|
||||||
|
test_expression->set_is_lhs(true);
|
||||||
/* Cache value of test expression. */
|
/* Cache value of test expression. */
|
||||||
ir_rvalue *const test_val =
|
ir_rvalue *const test_val =
|
||||||
test_expression->hir(instructions,
|
test_expression->hir(instructions,
|
||||||
|
@@ -1206,6 +1206,7 @@ ast_expression::ast_expression(int oper,
|
|||||||
this->subexpressions[1] = ex1;
|
this->subexpressions[1] = ex1;
|
||||||
this->subexpressions[2] = ex2;
|
this->subexpressions[2] = ex2;
|
||||||
this->non_lvalue_description = NULL;
|
this->non_lvalue_description = NULL;
|
||||||
|
this->is_lhs = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user