From: Luca Bruno <lucabru@src.gnome.org>
Date: Wed, 4 Jan 2012 22:59:34 +0000 (+0100)
Subject: Move BinaryExpression transformation to the CCodeTransformer
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=88dfb7994ee05882a915566370fa2daa32e6a79b;p=thirdparty%2Fvala.git

Move BinaryExpression transformation to the CCodeTransformer

Fixes https://gitlab.gnome.org/GNOME/vala/issues/534
---

diff --git a/codegen/valaccodetransformer.vala b/codegen/valaccodetransformer.vala
index 476a538c0..c9374d832 100644
--- a/codegen/valaccodetransformer.vala
+++ b/codegen/valaccodetransformer.vala
@@ -264,4 +264,57 @@ public class Vala.CCodeTransformer : CodeTransformer {
 		replacement = return_temp_access (result, expr.value_type, target_type, formal_target_type);
 		end_replace_expression (replacement);
 	}
+
+	public override void visit_binary_expression (BinaryExpression expr) {
+		var parent_statement = expr.parent_statement;
+		if (parent_statement == null) {
+			base.visit_binary_expression (expr);
+			return;
+		}
+
+		Expression replacement = null;
+		var target_type = copy_type (expr.target_type);
+		begin_replace_expression (expr);
+
+		if (context.analyzer.get_current_non_local_symbol (expr) is Block
+		    && (expr.operator == BinaryOperator.AND || expr.operator == BinaryOperator.OR)) {
+			var is_and = expr.operator == BinaryOperator.AND;
+			var result = b.add_temp_declaration (data_type ("bool"));
+			b.open_if (expr.left);
+			if (is_and) {
+				b.add_assignment (expression (result), expr.right);
+			} else {
+				b.add_expression (expression (@"$result = true"));
+			}
+			b.add_else ();
+			if (is_and) {
+				b.add_expression (expression (@"$result = false"));
+			} else {
+				b.add_assignment (expression (result), expr.right);
+			}
+			b.close ();
+			replacement = expression (result);
+		} else if (expr.operator == BinaryOperator.COALESCE) {
+			var result = b.add_temp_declaration (copy_type (expr.value_type, null, true), expr.left);
+
+			b.open_if (expression (@"$result == null"));
+			b.add_assignment (expression (result), expr.right);
+			b.close ();
+
+			replacement = return_temp_access (result, expr.value_type, target_type);
+		} else if (expr.operator == BinaryOperator.IN && !(expr.left.value_type.compatible (context.analyzer.int_type) && expr.right.value_type.compatible (context.analyzer.int_type)) && !(expr.right.value_type is ArrayType)) {
+			// neither enums nor array, it's contains()
+			var call = new MethodCall (new MemberAccess (expr.right, "contains", expr.source_reference), expr.source_reference);
+			call.add_argument (expr.left);
+			replacement = call;
+		}
+
+		if (replacement != null) {
+			replacement.target_type = target_type;
+			end_replace_expression (replacement);
+		} else {
+			end_replace_expression (null);
+			base.visit_binary_expression (expr);
+		}
+	}
 }
diff --git a/vala/valabinaryexpression.vala b/vala/valabinaryexpression.vala
index d6d0444d0..2dda5017f 100644
--- a/vala/valabinaryexpression.vala
+++ b/vala/valabinaryexpression.vala
@@ -140,138 +140,6 @@ public class Vala.BinaryExpression : Expression {
 
 		checked = true;
 
-		var insert_block = context.analyzer.get_current_block (this);
-
-		// some expressions are not in a block,
-		// for example, expressions in method contracts
-		if (context.analyzer.get_current_non_local_symbol (this) is Block
-		    && (operator == BinaryOperator.AND || operator == BinaryOperator.OR)) {
-			// convert conditional expression into if statement
-			// required for flow analysis and exception handling
-
-			var local = new LocalVariable (context.analyzer.bool_type.copy (), get_temp_name (), null, source_reference);
-			var decl = new DeclarationStatement (local, source_reference);
-
-			var right_stmt = new ExpressionStatement (new Assignment (new MemberAccess.simple (local.name, right.source_reference), right, AssignmentOperator.SIMPLE, right.source_reference), right.source_reference);
-
-			var stmt = new ExpressionStatement (new Assignment (new MemberAccess.simple (local.name, left.source_reference), new BooleanLiteral ((operator == BinaryOperator.OR), left.source_reference), AssignmentOperator.SIMPLE, left.source_reference), left.source_reference);
-
-			var true_block = new Block (source_reference);
-			var false_block = new Block (source_reference);
-
-			if (operator == BinaryOperator.AND) {
-				true_block.add_statement (right_stmt);
-				false_block.add_statement (stmt);
-			} else {
-				true_block.add_statement (stmt);
-				false_block.add_statement (right_stmt);
-			}
-
-			var if_stmt = new IfStatement (left, true_block, false_block, source_reference);
-
-			insert_statement (insert_block, decl);
-			insert_statement (insert_block, if_stmt);
-
-			decl.check (context);
-
-			if (!if_stmt.check (context)) {
-				error = true;
-				return false;
-			}
-
-			var ma = new MemberAccess.simple (local.name, source_reference);
-			ma.target_type = target_type;
-			ma.formal_target_type = formal_target_type;
-
-			parent_node.replace_expression (this, ma);
-
-			ma.check (context);
-
-			return true;
-		}
-
-		if (operator == BinaryOperator.COALESCE) {
-			if (!left.check (context)) {
-				error = true;
-				return false;
-			}
-
-			if (!right.check (context)) {
-				error = true;
-				return false;
-			}
-
-			DataType local_type = null;
-			bool cast_non_null = false;
-			if (left.value_type is NullType && right.value_type != null) {
-				Report.warning (left.source_reference, "left operand is always null");
-				local_type = right.value_type.copy ();
-				local_type.nullable = true;
-				if (!right.value_type.nullable) {
-					cast_non_null = true;
-				}
-			} else if (left.value_type != null) {
-				local_type = left.value_type.copy ();
-				if (right.value_type != null && right.value_type.value_owned) {
-					// value owned if either left or right is owned
-					local_type.value_owned = true;
-				}
-				if (context.experimental_non_null) {
-					if (!local_type.nullable) {
-						Report.warning (left.source_reference, "left operand is never null");
-						if (right.value_type != null && right.value_type.nullable) {
-							local_type.nullable = true;
-							cast_non_null = true;
-						}
-					} else if (right.value_type != null && !right.value_type.nullable) {
-						cast_non_null = true;
-					}
-				}
-			} else if (right.value_type != null) {
-				local_type = right.value_type.copy ();
-			}
-
-			var local = new LocalVariable (local_type, get_temp_name (), left, source_reference);
-			var decl = new DeclarationStatement (local, source_reference);
-
-			var right_stmt = new ExpressionStatement (new Assignment (new MemberAccess.simple (local.name, right.source_reference), right, AssignmentOperator.SIMPLE, right.source_reference), right.source_reference);
-
-			var true_block = new Block (source_reference);
-
-			true_block.add_statement (right_stmt);
-
-			var cond = new BinaryExpression (BinaryOperator.EQUALITY, new MemberAccess.simple (local.name, left.source_reference), new NullLiteral (source_reference), source_reference);
-
-			var if_stmt = new IfStatement (cond, true_block, null, source_reference);
-
-			insert_statement (insert_block, decl);
-			insert_statement (insert_block, if_stmt);
-
-			if (!decl.check (context)) {
-				error = true;
-				return false;
-			}
-
-			if (!if_stmt.check (context)) {
-				error = true;
-				return false;
-			}
-
-			var replace_expr = SemanticAnalyzer.create_temp_access (local, target_type);
-			if (cast_non_null && replace_expr.target_type != null) {
-				var cast = new CastExpression.non_null (replace_expr, source_reference);
-				cast.target_type = replace_expr.target_type.copy ();
-				cast.target_type.nullable = false;
-				replace_expr = cast;
-			}
-
-			parent_node.replace_expression (this, replace_expr);
-
-			replace_expr.check (context);
-
-			return true;
-		}
-
 		// enum-type inference
 		if (target_type != null && target_type.data_type is Enum
 		    && (operator == BinaryOperator.BITWISE_AND || operator == BinaryOperator.BITWISE_OR)) {
@@ -324,7 +192,19 @@ public class Vala.BinaryExpression : Expression {
 		right.target_type = right.value_type.copy ();
 		right.target_type.value_owned = false;
 
-		if (left.value_type.data_type == context.analyzer.string_type.data_type
+		if (operator == BinaryOperator.COALESCE) {
+			left.target_type.nullable = true;
+			left.target_type.value_owned = left.value_type.value_owned || right.value_type.value_owned;
+
+			if (left is NullLiteral) {
+				left.target_type = right.target_type.copy ();
+				left.target_type.nullable = true;
+			} else {
+				right.target_type = left.target_type.copy ();
+			}
+			value_type = left.target_type.copy ();
+			value_type.nullable = left.value_type.nullable && right.value_type.nullable;
+		} else if (left.value_type.data_type == context.analyzer.string_type.data_type
 		    && operator == BinaryOperator.PLUS) {
 			// string concatenation
 
@@ -537,15 +417,9 @@ public class Vala.BinaryExpression : Expression {
 					error = true;
 					return false;
 				}
-
-				var contains_call = new MethodCall (new MemberAccess (right, "contains", source_reference), source_reference);
-				contains_call.add_argument (left);
-				parent_node.replace_expression (this, contains_call);
-				return contains_call.check (context);
 			}
 
 			value_type = context.analyzer.bool_type;
-
 		} else {
 			error = true;
 			Report.error (source_reference, "internal error: unsupported binary operator");
@@ -567,13 +441,19 @@ public class Vala.BinaryExpression : Expression {
 	}
 
 	public override void get_defined_variables (Collection<Variable> collection) {
-		left.get_defined_variables (collection);
-		right.get_defined_variables (collection);
+		//FIXME Handled special in FlowAnalyzer
+		if (operator != BinaryOperator.AND && operator != BinaryOperator.OR) {
+			left.get_defined_variables (collection);
+			right.get_defined_variables (collection);
+		}
 	}
 
 	public override void get_used_variables (Collection<Variable> collection) {
-		left.get_used_variables (collection);
-		right.get_used_variables (collection);
+		//FIXME Handled special in FlowAnalyzer
+		if (operator != BinaryOperator.AND && operator != BinaryOperator.OR) {
+			left.get_used_variables (collection);
+			right.get_used_variables (collection);
+		}
 	}
 }
 
diff --git a/vala/valaflowanalyzer.vala b/vala/valaflowanalyzer.vala
index 8ebb5422b..a063f0494 100644
--- a/vala/valaflowanalyzer.vala
+++ b/vala/valaflowanalyzer.vala
@@ -619,8 +619,8 @@ public class Vala.FlowAnalyzer : CodeVisitor {
 
 		// condition
 		current_block.add_node (stmt.condition);
-
 		handle_errors (stmt.condition);
+		stmt.condition.accept (this);
 
 		// true block
 		var last_block = current_block;
@@ -771,6 +771,7 @@ public class Vala.FlowAnalyzer : CodeVisitor {
 		// condition
 		loop_block.add_node (stmt.condition);
 		handle_errors (stmt.condition);
+		stmt.condition.accept (this);
 
 		if (always_false (stmt.condition)) {
 			mark_unreachable ();
@@ -1260,7 +1261,67 @@ public class Vala.FlowAnalyzer : CodeVisitor {
 	public override void visit_expression (Expression expr) {
 		// lambda expression and conditional expression are handled separately
 		// an expression can be unreachable due to a conditional expression
-		if (!(expr is LambdaExpression) && !(expr is ConditionalExpression) && !unreachable (expr)) {
+		if (!(expr is LambdaExpression) && !(expr is ConditionalExpression)
+		    && !(expr is BinaryExpression) && !unreachable (expr)) {
+			expr.accept_children (this);
+		}
+	}
+
+	public override void visit_binary_expression (BinaryExpression expr) {
+		if (unreachable (expr)) {
+			return;
+		}
+
+		if (expr.operator == BinaryOperator.AND || expr.operator == BinaryOperator.OR) {
+			var is_and = expr.operator == BinaryOperator.AND;
+
+			// condition
+			current_block.add_node (expr.left);
+			handle_errors (expr.left);
+			expr.left.accept (this);
+
+			// true block
+			var last_block = current_block;
+			if (always_false (expr.left)) {
+				mark_unreachable ();
+			} else {
+				current_block = new BasicBlock ();
+				all_basic_blocks.add (current_block);
+				last_block.connect (current_block);
+			}
+			if (is_and) {
+				expr.right.accept (this);
+			}
+
+			// false block
+			var last_true_block = current_block;
+			if (always_true (expr.left)) {
+				mark_unreachable ();
+			} else {
+				current_block = new BasicBlock ();
+				all_basic_blocks.add (current_block);
+				last_block.connect (current_block);
+			}
+			if (!is_and) {
+				expr.right.accept (this);
+			}
+
+			// after if/else
+			var last_false_block = current_block;
+			// reachable?
+			if (last_true_block != null || last_false_block != null) {
+				current_block = new BasicBlock ();
+				all_basic_blocks.add (current_block);
+				if (last_true_block != null) {
+					last_true_block.connect (current_block);
+				}
+				if (last_false_block != null) {
+					last_false_block.connect (current_block);
+				}
+			}
+		} else if (expr.operator == BinaryOperator.COALESCE) {
+			//TODO
+		} else {
 			expr.accept_children (this);
 		}
 	}
@@ -1281,6 +1342,7 @@ public class Vala.FlowAnalyzer : CodeVisitor {
 		// condition
 		current_block.add_node (expr.condition);
 		handle_errors (expr.condition);
+		expr.condition.accept (this);
 
 		// true block
 		var last_block = current_block;