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

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 314b01439abc67d5aa91746bad9338e2eb3247c6..0d4968e75d5e942f26de1daf8d8de9d9536818e1 100644 (file)
--- 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 (file)
index 0000000..b616e9c
--- /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 (file)
index 0000000..f5404e5
--- /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 (file)
index 0000000..167b454
--- /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 927ca541383735299601e1a658bd625041aaf0dd..7056350334d02a91b32b0277567da5162740e59c 100644 (file)
--- 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;