From 6f673c2c9768402cbfe5e206da9c3c0180e5a4b3 Mon Sep 17 00:00:00 2001 From: Jeremy Philippe Date: Sat, 11 Jan 2020 01:18:43 +0100 Subject: [PATCH] vala: Fix short-circuiting behavior of coalescing operator It is closely modeled after how ConditionalExpression implements short-circuiting behavior. Fixes https://gitlab.gnome.org/GNOME/vala/issues/534 --- tests/Makefile.am | 3 ++ tests/control-flow/bug764440.vala | 16 ++++++++ .../coalesce-execution-order.vala | 19 ++++++++++ .../control-flow/coalesce-short-circuit.vala | 14 +++++++ vala/valabinaryexpression.vala | 37 ++++++++++++++----- 5 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 tests/control-flow/bug764440.vala create mode 100644 tests/control-flow/coalesce-execution-order.vala create mode 100644 tests/control-flow/coalesce-short-circuit.vala diff --git a/tests/Makefile.am b/tests/Makefile.am index 314b01439..0d4968e75 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -165,8 +165,10 @@ TESTS = \ control-flow/assigned-local-variable.vala \ control-flow/break.vala \ control-flow/break-invalid.test \ + control-flow/coalesce-execution-order.vala \ control-flow/coalesce-reference-transfer.vala \ control-flow/coalesce-right-value.vala \ + control-flow/coalesce-short-circuit.vala \ control-flow/continue-invalid.test \ control-flow/double-catch.test \ control-flow/expressions-conditional.vala \ @@ -194,6 +196,7 @@ TESTS = \ control-flow/bug691514.vala \ control-flow/bug736774-1.vala \ control-flow/bug736774-2.vala \ + control-flow/bug764440.vala \ control-flow/bug790903.test \ control-flow/bug790903-2.test \ control-semantic/argument-extra.test \ diff --git a/tests/control-flow/bug764440.vala b/tests/control-flow/bug764440.vala new file mode 100644 index 000000000..b616e9c21 --- /dev/null +++ b/tests/control-flow/bug764440.vala @@ -0,0 +1,16 @@ +errordomain FooError { + BAR; +} + +unowned string get_bar () throws FooError { + throw new FooError.BAR ("bar"); +} + +void main () { + try { + unowned string? foo = "foo"; + unowned string bar = foo ?? get_bar (); + } catch { + assert_not_reached (); + } +} diff --git a/tests/control-flow/coalesce-execution-order.vala b/tests/control-flow/coalesce-execution-order.vala new file mode 100644 index 000000000..f5404e57f --- /dev/null +++ b/tests/control-flow/coalesce-execution-order.vala @@ -0,0 +1,19 @@ +string? get_foo () { + assert (count == 0); + return null; +} + +string get_bar () { + count++; + assert (count == 1); + return "bar"; +} + +int count; + +void main () { + count = 0; + string? s = null; + string foo = s ?? get_foo () ?? get_bar (); + assert (foo == "bar"); +} diff --git a/tests/control-flow/coalesce-short-circuit.vala b/tests/control-flow/coalesce-short-circuit.vala new file mode 100644 index 000000000..167b454fa --- /dev/null +++ b/tests/control-flow/coalesce-short-circuit.vala @@ -0,0 +1,14 @@ +string get_foo () { + assert_not_reached (); +} + +void main () { + { + string foo = "foo" ?? get_foo (); + assert (foo == "foo"); + } + { + string foo = "foo" ?? get_foo () ?? get_foo (); + assert (foo == "foo"); + } +} diff --git a/vala/valabinaryexpression.vala b/vala/valabinaryexpression.vala index 927ca5413..705635033 100644 --- a/vala/valabinaryexpression.vala +++ b/vala/valabinaryexpression.vala @@ -199,11 +199,22 @@ public class Vala.BinaryExpression : Expression { return false; } - if (!right.check (context)) { + 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) { @@ -240,27 +251,33 @@ public class Vala.BinaryExpression : Expression { local_type.value_owned = true; } - var local = new LocalVariable (local_type, get_temp_name (), left, source_reference); + var local = new LocalVariable (local_type, temp_name, left, source_reference); var decl = new DeclarationStatement (local, source_reference); + insert_statement (context.analyzer.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); - var true_block = new Block (source_reference); + true_block.remove_local_variable (right_local); + true_block.replace_statement (right_decl, right_stmt); - true_block.add_statement (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 (context.analyzer.insert_block, decl); insert_statement (context.analyzer.insert_block, if_stmt); - if (!decl.check (context)) { - error = true; - return false; - } - if (!if_stmt.check (context)) { error = true; return false; -- 2.47.2