Avoid exposing EnsoMultiValue getters (#11642)

Another step in the #11482 work. Avoid accessing internals of `EnsoMultiValue`. Use `TypeOfNode` methods (as provided by #11618) instead.
enso-org · Nov 26, 2024 · 1ea4d1a · 1ea4d1a
1 parent cb8d5c0
commit 1ea4d1a
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 6 deletions.
diff --git a/.../src/test/java/org/enso/interpreter/node/expression/builtin/meta/TypeOfNodeValueTest.java b/.../src/test/java/org/enso/interpreter/node/expression/builtin/meta/TypeOfNodeValueTest.java
@@ -0,0 +1,82 @@
+package org.enso.interpreter.node.expression.builtin.meta;
+
+import static org.junit.Assert.assertEquals;
+
+import com.oracle.truffle.api.RootCallTarget;
+import org.enso.interpreter.runtime.callable.UnresolvedConstructor;
+import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
+import org.enso.interpreter.runtime.data.Type;
+import org.enso.interpreter.runtime.library.dispatch.TypeOfNode;
+import org.enso.test.utils.ContextUtils;
+import org.enso.test.utils.TestRootNode;
+import org.graalvm.polyglot.Context;
+import org.junit.AfterClass;
+import org.junit.Test;
+
+public class TypeOfNodeValueTest {
+  private static RootCallTarget testTypesCall;
+  private static Context ctx;
+
+  private static Context ctx() {
+    if (ctx == null) {
+      ctx = ContextUtils.defaultContextBuilder().build();
+      ContextUtils.executeInContext(
+          ctx,
+          () -> {
+            var node = TypeOfNode.create();
+            var root =
+                new TestRootNode(
+                    (frame) -> {
+                      var arg = frame.getArguments()[0];
+                      var t = node.findTypeOrError(arg);
+                      var all = node.findAllTypesOrNull(arg);
+                      return new Object[] {t, all};
+                    });
+            root.insertChildren(node);
+            testTypesCall = root.getCallTarget();
+            return null;
+          });
+    }
+    return ctx;
+  }
+
+  @AfterClass
+  public static void disposeCtx() throws Exception {
+    if (ctx != null) {
+      ctx.close();
+      ctx = null;
+    }
+  }
+
+  @Test
+  public void typeOfUnresolvedConstructor() {
+    ContextUtils.executeInContext(
+        ctx(),
+        () -> {
+          var cnstr = UnresolvedConstructor.build(null, "Unknown_Name");
+          var arr = (Object[]) testTypesCall.call(cnstr);
+          var type = (Type) arr[0];
+          var allTypes = (Type[]) arr[1];
+          assertEquals("Function", type.getName());
+          assertEquals("One array", 1, allTypes.length);
+          assertEquals("Also function type", type, allTypes[0]);
+          return null;
+        });
+  }
+
+  @Test
+  public void typeOfUnresolvedSymbol() {
+    ContextUtils.executeInContext(
+        ctx(),
+        () -> {
+          var cnstr = UnresolvedSymbol.build("Unknown_Name", null);
+          var arr = (Object[]) testTypesCall.call(cnstr);
+          var type = (Type) arr[0];
+          var allTypes = (Type[]) arr[1];
+          assertEquals("Function", type.getName());
+          assertEquals("One array", 1, allTypes.length);
+          assertEquals("Also function type", type, allTypes[0]);
+          return null;
+        });
+  }
+}
diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/BinaryOperatorNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/BinaryOperatorNode.java
@@ -152,7 +152,9 @@ final Object doThatConversionUncached(
             InvokeFunctionNode invokeNode) {
       var selfType = findType(typeOfNode, self);
       if (that instanceof EnsoMultiValue multi) {
-        for (var thatType : multi.allTypes()) {
+        var all = typeOfNode.findAllTypesOrNull(multi);
+        assert all != null;
+        for (var thatType : all) {
           var fn = findSymbol(symbol, thatType);
           if (fn != null) {
             var thatCasted = EnsoMultiValue.CastToNode.getUncached().executeCast(thatType, multi);

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/typecheck/SingleTypeCheckNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/typecheck/SingleTypeCheckNode.java
@@ -173,7 +173,9 @@ Type[] findType(TypeOfNode typeOfNode, Object v) {
 
   Type[] findType(TypeOfNode typeOfNode, Object v, Type[] previous) {
     if (v instanceof EnsoMultiValue multi) {
-      return multi.allTypes();
+      var all = typeOfNode.findAllTypesOrNull(multi);
+      assert all != null;
+      return all;
     }
     if (v instanceof UnresolvedConstructor) {
       return null;

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoMultiValue.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoMultiValue.java
@@ -25,6 +25,7 @@
 import java.util.Arrays;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.enso.interpreter.node.callable.resolver.MethodResolverNode;
 import org.enso.interpreter.runtime.EnsoContext;
 import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
@@ -47,6 +48,8 @@ private EnsoMultiValue(Type[] types, Object[] values) {
     this.types = types;
     assert types.length == values.length;
     this.values = values;
+    assert !Stream.of(values).filter(v -> v instanceof EnsoMultiValue).findAny().isPresent()
+        : "Avoid double wrapping " + Arrays.toString(values);
   }
 
   public static EnsoObject create(Type[] types, Object[] values) {
@@ -64,19 +67,19 @@ boolean hasSpecialDispatch() {
   }
 
   @ExportMessage
-  public final Type getType() {
+  final Type getType() {
     return types[0];
   }
 
   @ExportMessage
-  public final Type[] allTypes() {
+  final Type[] allTypes() {
     return types.clone();
   }
 
   @ExportMessage
   @TruffleBoundary
   @Override
-  public String toDisplayString(boolean ignore) {
+  public final String toDisplayString(boolean ignore) {
     return toString();
   }
 

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/error/PanicException.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/error/PanicException.java
@@ -137,7 +137,7 @@ static UnresolvedSymbol toDisplayText(Object payload, IndirectInvokeMethodNode p
       }
     } catch (Error | Exception e) {
       logger().atError().log("Cannot compute message for " + payload, e);
-      throw UnsupportedMessageException.create(e);
+      throw UnsupportedMessageException.create(e instanceof AbstractTruffleException ? e : null);
     }
     var scope = ctx.getBuiltins().panic().getDefinitionScope();
     return UnresolvedSymbol.build("to_display_text", scope);