From 421e3f22a4b52487d309fa5f888a0fefbbbf2125 Mon Sep 17 00:00:00 2001
From: Jaroslav Tulach <jaroslav.tulach@enso.org>
Date: Wed, 4 Oct 2023 12:31:06 +0200
Subject: [PATCH] Avoid unnecessary conversions (#7952)

Fixes #7854 by checking all elements of a union type for direct match and only then, if direct match isn't available, applying conversions.
---
 .../argument/ReadArgumentCheckNode.java       | 16 +++++++---
 test/Tests/src/Semantic/Conversion_Spec.enso  | 30 +++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java
index 3305cff1940c..338c1597b70b 100644
--- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java
+++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java
@@ -187,6 +187,14 @@ static final class OneOfNode extends ReadArgumentCheckNode {
     @Override
     @ExplodeLoop
     Object executeCheckOrConversion(VirtualFrame frame, Object value) {
+      for (var n : checks) {
+        if (n instanceof TypeCheckNode typeCheck) {
+          var result = typeCheck.findAmongTypes(value);
+          if (result != null) {
+            return result;
+          }
+        }
+      }
       for (var n : checks) {
         var result = n.executeCheckOrConversion(frame, value);
         if (result != null) {
@@ -218,11 +226,11 @@ static abstract class TypeCheckNode extends ReadArgumentCheckNode {
     }
 
     @Specialization
-  Object doPanicSentinel(VirtualFrame frame, PanicSentinel panicSentinel) {
-    throw panicSentinel;
-  }
+    Object doPanicSentinel(VirtualFrame frame, PanicSentinel panicSentinel) {
+      throw panicSentinel;
+    }
 
-  @Specialization(rewriteOn = InvalidAssumptionException.class)
+    @Specialization(rewriteOn = InvalidAssumptionException.class)
     Object doCheckNoConversionNeeded(VirtualFrame frame, Object v) throws InvalidAssumptionException {
       var ret = findAmongTypes(v);
       if (ret != null) {
diff --git a/test/Tests/src/Semantic/Conversion_Spec.enso b/test/Tests/src/Semantic/Conversion_Spec.enso
index 12fe3cec3013..ced53dbfa685 100644
--- a/test/Tests/src/Semantic/Conversion_Spec.enso
+++ b/test/Tests/src/Semantic/Conversion_Spec.enso
@@ -47,6 +47,21 @@ Integer.from (that:MultiNumber) = that.v * 19
 Number.from (that:MultiNumber) = that.v * 0.3
 Float.from (that:MultiNumber) = that.v * 0.7
 
+type Back
+    Times n:Integer
+
+    exchange : (Back | Forth) -> (Back | Forth)
+    exchange value:(Back | Forth) = value
+
+type Forth
+    Times n:Integer
+
+    exchange : (Forth | Back) -> (Forth | Back)
+    exchange value:(Forth | Back) = value
+
+Back.from (that:Forth) = Back.Times that.n+1
+Forth.from (that:Back) = Forth.Times that.n+1
+
 foreign js make_str x = """
    return "js string"
 
@@ -201,6 +216,21 @@ spec =
             once . should_equal second
             Meta.is_same_object once second . should_be_true
 
+        Test.specify "Avoid back and forth conversions" <|
+            one = Forth.Times 1
+
+            two = Back.exchange one
+            three = Forth.exchange two
+            four = Back.exchange three
+            five = Forth.exchange four
+            six = Forth.exchange five
+            seven = Back.exchange six
+            eight = Back.exchange seven
+            nine = Forth.exchange eight
+
+            # no conversions needed when calling `exchange` methods
+            nine . should_equal one
+
         Test.specify "Requesting Text & Foo" <|
             check a (n : Text & Foo) = case a of
                 0 -> n.foo