[vala] Fix side-effects in assignments

[Jürg Billeter <juergbi src gnome org>]

commit b677fffd9ef9597df389b8636a02ef02209df378
Author: Jürg Billeter <j bitron ch>
Date:   Mon Mar 30 12:10:34 2009 +0200

    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.
---
 gobject/valaccodeassignmentmodule.vala |   43 ++++++++++++++++---------------
 gobject/valaccodebasemodule.vala       |   34 +++++++++++++++++++++---
 tests/Makefile.am                      |    1 +
 tests/control-flow/sideeffects.test    |   21 +++++++++++++++
 4 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/gobject/valaccodeassignmentmodule.vala b/gobject/valaccodeassignmentmodule.vala
index c1d0832..332bb4f 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 ee3983e..9bfa75c 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 a7bf6aa..6e113ee 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 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;
+	}
+}