[vala/staging] vala: Fix short-circuiting behavior of coalescing operator

[Rico Tzschichholz <ricotz src gnome org>]

commit 6f673c2c9768402cbfe5e206da9c3c0180e5a4b3
Author: Jeremy Philippe <jeremy philippe gmail com>
Date:   Sat Jan 11 01:18:43 2020 +0100

    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 ++++++++++
 tests/control-flow/coalesce-execution-order.vala | 19 ++++++++++++
 tests/control-flow/coalesce-short-circuit.vala   | 14 +++++++++
 vala/valabinaryexpression.vala                   | 37 +++++++++++++++++-------
 5 files changed, 79 insertions(+), 10 deletions(-)
---
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;