enso-org / enso

Hybrid visual and textual functional programming.
https://enso.org
Apache License 2.0
7.3k stars 317 forks source link

Comparable.from doesn't return Comparable #10355

Open JaroslavTulach opened 3 days ago

JaroslavTulach commented 3 days ago

Alas, conversion methods are not checked right now, because Comparable.from typically violates the required contract - it doesn't return Comparable, but some unrelated type.

The goal of this issue is to change the way we deal with Comparable and turn on the return type check for conversion methods.

type Comparable
    private Both value:Any comparator:Any

    new value:Any comparator:Any -> Comparable = Comparable.Both value comparator

    // implement operations by using self.value and that.value and delegating to self.comparator
    <,>,>=,<=,=
JaroslavTulach commented 3 days ago

This is the code that enables return type check for conversion methods:

diff --git engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExecCompilerTest.java engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExecCompilerTest.java
index c194132011..ad4ea01208 100644
--- engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExecCompilerTest.java
+++ engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExecCompilerTest.java
@@ -21,6 +21,7 @@ import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.PolyglotException;
 import org.graalvm.polyglot.Source;
 import org.graalvm.polyglot.io.IOAccess;
+import org.hamcrest.core.AllOf;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Ignore;
@@ -478,4 +479,30 @@ public class ExecCompilerTest {
     runMethod.execute(0);
     assertThat(out.toString(), containsString(expectedErrMsg));
   }
+
+  @Test
+  public void resultOfConversionIsTypeChecked() throws Exception {
+    var module =
+        ctx.eval(
+            LanguageInfo.ID,
+            """
+        type First_Type
+        type Other_Type
+
+        First_Type.from (that:Other_Type) = 42
+        run value -> First_Type = Other_Type
+        """);
+    var runMethod = module.invokeMember(Module.EVAL_EXPRESSION, "run");
+    try {
+      var r = runMethod.execute(0);
+      fail("We don't expect any result, but exception: " + r);
+    } catch (PolyglotException ex) {
+      assertThat(
+          ex.getMessage().toLowerCase(),
+          AllOf.allOf(containsString("type"), containsString("error")));
+      var typeError = ex.getGuestObject();
+      assertEquals("Expected type", "First_Type", typeError.getMember("expected").toString());
+      assertEquals("Got wrong value", 42, typeError.getMember("actual").asInt());
+    }
+  }
 }
diff --git engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
index fc6ddf9f78..606c839864 100644
--- engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
+++ engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
@@ -504,6 +504,7 @@ class IrToTruffle(
                             m.getFunction.getName,
                             fn.arguments,
                             fn.body,
+                            null,
                             effectContext,
                             true
                           )
@@ -536,6 +537,7 @@ class IrToTruffle(
                     fullMethodDefName,
                     fn.arguments,
                     fn.body,
+                    null,
                     effectContext,
                     true
                   )
@@ -712,6 +714,7 @@ class IrToTruffle(
                 methodDef.methodName.name,
                 fn.arguments,
                 fn.body,
+                ReadArgumentCheckNode.build(context, "conversion", toType),
                 None,
                 true
               )
@@ -1921,6 +1924,7 @@ class IrToTruffle(
       val initialName: String,
       val arguments: List[DefinitionArgument],
       val body: Expression,
+      val typeCheck: ReadArgumentCheckNode,
       val effectContext: Option[String],
       val subjectToInstrumentation: Boolean
     ) {
@@ -1949,7 +1953,13 @@ class IrToTruffle(
           case _ =>
             ExpressionProcessor.this.run(body, false, subjectToInstrumentation)
         }
-        (argExpressions.toArray, bodyExpr)
+
+        if (typeCheck == null) {
+          (argExpressions.toArray, bodyExpr)
+        } else {
+          val bodyWithCheck = ReadArgumentCheckNode.wrap(bodyExpr, typeCheck)
+          (argExpressions.toArray, bodyWithCheck)
+        }
       }

       private def computeSlots(): (
@@ -2039,7 +2049,7 @@ class IrToTruffle(
       binding: Boolean = false
     ): CreateFunctionNode = {
       val bodyBuilder =
-        new BuildFunctionBody(scopeName, arguments, body, None, false)
+        new BuildFunctionBody(scopeName, arguments, body, null, None, false)
       val fnRootNode = ClosureRootNode.build(
         language,
         scope,