From: Jürg Billeter Date: Mon, 30 Mar 2009 10:10:34 +0000 (+0200) Subject: Fix side-effects in assignments X-Git-Tag: 0.6.0~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b677fffd9ef9597df389b8636a02ef02209df378;p=thirdparty%2Fvala.git Fix side-effects in assignments Do not evaluate the left-hand side of an assignment multiple times if it could have side-effects. Based on patch by Levi Bard, fixes bug 543483. --- diff --git a/gobject/valaccodeassignmentmodule.vala b/gobject/valaccodeassignmentmodule.vala index c1d0832d3..332bb4f03 100644 --- a/gobject/valaccodeassignmentmodule.vala +++ b/gobject/valaccodeassignmentmodule.vala @@ -94,6 +94,8 @@ internal class Vala.CCodeAssignmentModule : CCodeMemberAccessModule { CCodeExpression emit_simple_assignment (Assignment assignment) { CCodeExpression rhs = (CCodeExpression) assignment.right.ccodenode; + CCodeExpression lhs = (CCodeExpression) get_ccodenode (assignment.left); + CCodeCommaExpression outer_ccomma = null; bool unref_old = requires_destroy (assignment.left.value_type); bool array = false; @@ -105,16 +107,28 @@ internal class Vala.CCodeAssignmentModule : CCodeMemberAccessModule { var delegate_type = (DelegateType) assignment.left.value_type; instance_delegate = delegate_type.delegate_symbol.has_target; } - + if (unref_old || array || instance_delegate) { var ccomma = new CCodeCommaExpression (); - + + if (!is_pure_ccode_expression (lhs)) { + /* Assign lhs to temp var to avoid repeating side effect */ + outer_ccomma = new CCodeCommaExpression (); + + var lhs_value_type = assignment.left.value_type.copy (); + string lhs_temp_name = "_tmp%d".printf (next_temp_var_id++); + var lhs_temp = new LocalVariable (lhs_value_type, "*" + lhs_temp_name); + temp_vars.insert (0, lhs_temp); + outer_ccomma.append_expression (new CCodeAssignment (get_variable_cexpression (lhs_temp_name), new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, lhs))); + lhs = new CCodeParenthesizedExpression (new CCodeUnaryExpression (CCodeUnaryOperator.POINTER_INDIRECTION, get_variable_cexpression (lhs_temp_name))); + } + var temp_decl = get_temp_variable (assignment.left.value_type); temp_vars.insert (0, temp_decl); ccomma.append_expression (new CCodeAssignment (get_variable_cexpression (temp_decl.name), rhs)); if (unref_old) { /* unref old value */ - ccomma.append_expression (get_unref_expression ((CCodeExpression) get_ccodenode (assignment.left), assignment.left.value_type, assignment.left)); + ccomma.append_expression (get_unref_expression (lhs, assignment.left.value_type, assignment.left)); } if (array) { @@ -166,25 +180,12 @@ internal class Vala.CCodeAssignmentModule : CCodeMemberAccessModule { } else if (assignment.operator == AssignmentOperator.SHIFT_RIGHT) { cop = CCodeAssignmentOperator.SHIFT_RIGHT; } - - CCodeExpression codenode = new CCodeAssignment ((CCodeExpression) get_ccodenode (assignment.left), rhs, cop); - - if (unref_old && get_ccodenode (assignment.left) is CCodeElementAccess) { - // ensure that index expression in element access doesn't get evaluated more than once - // except when it's a simple expression - var cea = (CCodeElementAccess) get_ccodenode (assignment.left); - if (!(cea.index is CCodeConstant || cea.index is CCodeIdentifier)) { - var index_temp_decl = get_temp_variable (int_type); - temp_vars.insert (0, index_temp_decl); - - var ccomma = new CCodeCommaExpression (); - ccomma.append_expression (new CCodeAssignment (get_variable_cexpression (index_temp_decl.name), cea.index)); - ccomma.append_expression (codenode); - cea.index = get_variable_cexpression (index_temp_decl.name); - - codenode = ccomma; - } + CCodeExpression codenode = new CCodeAssignment (lhs, rhs, cop); + + if (outer_ccomma != null) { + outer_ccomma.append_expression (codenode); + codenode = outer_ccomma; } return codenode; diff --git a/gobject/valaccodebasemodule.vala b/gobject/valaccodebasemodule.vala index ee3983e1f..9bfa75c75 100644 --- a/gobject/valaccodebasemodule.vala +++ b/gobject/valaccodebasemodule.vala @@ -1073,13 +1073,32 @@ internal class Vala.CCodeBaseModule : CCodeModule { } else if (cexpr is CCodeBinaryExpression) { var cbinary = (CCodeBinaryExpression) cexpr; return is_pure_ccode_expression (cbinary.left) && is_constant_ccode_expression (cbinary.right); + } else if (cexpr is CCodeUnaryExpression) { + var cunary = (CCodeUnaryExpression) cexpr; + switch (cunary.operator) { + case CCodeUnaryOperator.PREFIX_INCREMENT: + case CCodeUnaryOperator.PREFIX_DECREMENT: + case CCodeUnaryOperator.POSTFIX_INCREMENT: + case CCodeUnaryOperator.POSTFIX_DECREMENT: + return false; + default: + return is_pure_ccode_expression (cunary.inner); + } } else if (cexpr is CCodeMemberAccess) { var cma = (CCodeMemberAccess) cexpr; return is_pure_ccode_expression (cma.inner); + } else if (cexpr is CCodeElementAccess) { + var cea = (CCodeElementAccess) cexpr; + return is_pure_ccode_expression (cea.container) && is_pure_ccode_expression (cea.index); + } else if (cexpr is CCodeCastExpression) { + var ccast = (CCodeCastExpression) cexpr; + return is_pure_ccode_expression (ccast.inner); + } else if (cexpr is CCodeParenthesizedExpression) { + var cparenthesized = (CCodeParenthesizedExpression) cexpr; + return is_pure_ccode_expression (cparenthesized.inner); } - var cparenthesized = (cexpr as CCodeParenthesizedExpression); - return (null != cparenthesized && is_pure_ccode_expression (cparenthesized.inner)); + return false; } public override void visit_formal_parameter (FormalParameter p) { @@ -2013,7 +2032,12 @@ internal class Vala.CCodeBaseModule : CCodeModule { var st = local.variable_type.data_type as Struct; - if (local.variable_type.is_reference_type_or_type_parameter ()) { + if (local.name.has_prefix ("*")) { + // do not dereference unintialized variable + // initialization is not needed for these special + // pointer temp variables + // used to avoid side-effects in assignments + } else if (local.variable_type.is_reference_type_or_type_parameter ()) { vardecl.initializer = new CCodeConstant ("NULL"); } else if (st != null && !st.is_simple_type ()) { // 0-initialize struct with struct initializer { 0 } diff --git a/tests/Makefile.am b/tests/Makefile.am index a7bf6aa52..6e113eeea 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -26,6 +26,7 @@ TESTS = \ control-flow/expressions-conditional.test \ control-flow/for.test \ control-flow/switch.test \ + control-flow/sideeffects.test \ enums/enums.test \ structs/structs.test \ delegates/delegates.test \ diff --git a/tests/control-flow/sideeffects.test b/tests/control-flow/sideeffects.test new file mode 100644 index 000000000..a334d3323 --- /dev/null +++ b/tests/control-flow/sideeffects.test @@ -0,0 +1,21 @@ + +Program: test + +class Maman.Foo : Object { + public int i = 1; + + public weak Foo sideeffect () { + --i; + return this; + } + public string data; +} + +class Maman.Bar : Object { + static int main (string[] args) { + var foo = new Foo (); + foo.sideeffect ().data = "foo"; + assert (foo.i == 0); + return 0; + } +}