From ed231394e9b34c1faf23da711afe194120ad5353 Mon Sep 17 00:00:00 2001 From: Luca Bruno Date: Wed, 4 Jan 2012 23:59:34 +0100 Subject: [PATCH] Move BinaryExpression transformation to the code transformer --- codegen/valaccodetransformer.vala | 55 +++++++++ vala/valabinaryexpression.vala | 196 ++++-------------------------- vala/valaflowanalyzer.vala | 66 +++++++++- 3 files changed, 146 insertions(+), 171 deletions(-) diff --git a/codegen/valaccodetransformer.vala b/codegen/valaccodetransformer.vala index c89190e4a..1eac04ade 100644 --- a/codegen/valaccodetransformer.vala +++ b/codegen/valaccodetransformer.vala @@ -243,4 +243,59 @@ 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_type = copy_type (expr.value_type); + result_type.nullable = true; + var result = b.add_temp_declaration (result_type, expr.left); + + b.open_if (expression (@"$result == null")); + b.add_assignment (expression (result), expr.right); + b.close (); + + replacement = return_temp_access (result, result_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 331aa0d55..c446a8ccc 100644 --- a/vala/valabinaryexpression.vala +++ b/vala/valabinaryexpression.vala @@ -140,166 +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 (target_type != null) { - left.target_type = target_type.copy (); - right.target_type = target_type.copy (); - } - - if (!left.check (context)) { - error = true; - return false; - } - - string temp_name = get_temp_name (); - - // right expression is checked under a block (required for short-circuiting) - var right_local = new LocalVariable (null, temp_name, right, right.source_reference); - var right_decl = new DeclarationStatement (right_local, right.source_reference); - var true_block = new Block (source_reference); - true_block.add_statement (right_decl); - - if (!true_block.check (context)) { - error = true; - return false; - } - - // right expression may have been replaced by the check - right = right_local.initializer; - - 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 (); - } - - if (local_type != null && right.value_type is ValueType && !right.value_type.nullable) { - // immediate values in the right expression must always be boxed, - // otherwise the local variable may receive a stale pointer to the stack - local_type.value_owned = true; - } - - var local = new LocalVariable (local_type, temp_name, left, source_reference); - var decl = new DeclarationStatement (local, source_reference); - - insert_statement (insert_block, decl); - - if (!decl.check (context)) { - error = true; - return false; - } - - // replace the temporary local variable used to compute the type of the right expression by an assignment - var right_stmt = new ExpressionStatement (new Assignment (new MemberAccess.simple (local.name, right.source_reference), right, AssignmentOperator.SIMPLE, right.source_reference), right.source_reference); - - true_block.remove_local_variable (right_local); - true_block.replace_statement (right_decl, right_stmt); - - if (!right_stmt.check (context)) { - error = true; - return false; - } - - 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, if_stmt); - - 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.type_symbol is Enum && (operator == BinaryOperator.BITWISE_AND || operator == BinaryOperator.BITWISE_OR)) { @@ -538,6 +378,23 @@ public class Vala.BinaryExpression : Expression { value_type = context.analyzer.bool_type; break; + case BinaryOperator.COALESCE: + if (left.target_type is NullType && right.value_type != null) { + Report.warning (left.source_reference, "left operand is always null"); + left.target_type = right.value_type.copy (); + } + left.target_type.nullable = true; + left.target_type.value_owned = left.value_type.value_owned || right.value_type.value_owned; + if (right.value_type is ValueType && !right.value_type.nullable) { + // immediate values in the right expression must always be boxed, + // otherwise the local variable may receive a stale pointer to the stack + left.target_type.value_owned = true; + } + right.target_type = left.target_type.copy (); + + value_type = left.target_type.copy (); + value_type.nullable = left.value_type.nullable && right.value_type.nullable; + break; case BinaryOperator.BITWISE_AND: case BinaryOperator.BITWISE_OR: case BinaryOperator.BITWISE_XOR: @@ -593,11 +450,6 @@ 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; @@ -623,13 +475,19 @@ public class Vala.BinaryExpression : Expression { } public override void get_defined_variables (Collection 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 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 ad6ef8ad6..c3531359e 100644 --- a/vala/valaflowanalyzer.vala +++ b/vala/valaflowanalyzer.vala @@ -606,8 +606,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; @@ -758,6 +758,7 @@ public class Vala.FlowAnalyzer : CodeVisitor { // condition loop_block.add_node (stmt.condition); handle_errors (stmt.condition); + stmt.condition.accept (this); if (stmt.condition.is_always_false ()) { mark_unreachable (); @@ -1249,7 +1250,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 (expr.left.is_always_false ()) { + 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 (expr.left.is_always_true ()) { + 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); } } @@ -1262,6 +1323,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; -- 2.47.2