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 c1d0832d3e69296d7ad73a010e915151bb8bcd67..332bb4f034d7003a9ae1eeb93ba1f06d64191604 100644 (file)
--- 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 ee3983e1f1ddc8c085c90bc85ee47cc7ae2e3afb..9bfa75c75fc121d0ecdd9f281cf353008d118ffe 100644 (file)
--- 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 a7bf6aa52a6098699cb75648bd77782f51c2fe36..6e113eeea612555cfc00f5492a22dff7d97bf510 100644 (file)
--- 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 (file)
index 0000000..a334d33
--- /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;
+       }
+}