Invoke instance methods for Any overrides

This change modifies method dispatch for methods that override Any's definitions. When an overrided method is invoked statically we call Any's method to stay consistent. This change primarily addresses the plethora of problems related to `to_text` invocations. It does not attempt to completely modify method dispatch logic. Closes #6300.
enso-org · Apr 26, 2023 · 67c3ffb · 67c3ffb
1 parent b8f075a
commit 67c3ffb
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 18 deletions.
diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeMethodNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeMethodNode.java
@@ -11,10 +11,12 @@
 import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.nodes.RootNode;
 import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import com.oracle.truffle.api.source.SourceSection;
 import org.enso.interpreter.node.BaseNode;
+import org.enso.interpreter.node.MethodRootNode;
 import org.enso.interpreter.node.callable.dispatch.InvokeFunctionNode;
 import org.enso.interpreter.node.callable.resolver.HostMethodCallNode;
 import org.enso.interpreter.node.callable.resolver.MethodResolverNode;
@@ -102,19 +104,40 @@ public void setTailStatus(TailStatus tailStatus) {
   public abstract Object execute(
       VirtualFrame frame, State state, UnresolvedSymbol symbol, Object self, Object[] arguments);
 
-  @Specialization(guards = {"dispatch.hasType(self)", "!dispatch.hasSpecialDispatch(self)"})
+  @Specialization(guards = {"typesLibrary.hasType(self)", "!typesLibrary.hasSpecialDispatch(self)"})
   Object doFunctionalDispatch(
       VirtualFrame frame,
       State state,
       UnresolvedSymbol symbol,
       Object self,
       Object[] arguments,
-      @CachedLibrary(limit = "10") TypesLibrary dispatch,
+      @CachedLibrary(limit = "10") TypesLibrary typesLibrary,
       @Cached MethodResolverNode methodResolverNode) {
-    Function function = methodResolverNode.expectNonNull(self, dispatch.getType(self), symbol);
+
+    Type selfTpe = typesLibrary.getType(self);
+    Function function = methodResolverNode.expectNonNull(self, selfTpe, symbol);
+
+    RootNode where = function.getCallTarget().getRootNode();
+    if (where instanceof MethodRootNode node && typeCanOverride(node, EnsoContext.get(this))) {
+      Function anyFun = symbol.getScope().lookupMethodDefinition(EnsoContext.get(this).getBuiltins().any(), symbol.getName());
+      if (anyFun != null) {
+        function = anyFun;
+      }
+    }
     return invokeFunctionNode.execute(function, frame, state, arguments);
   }
 
+  private boolean typeCanOverride(MethodRootNode node, EnsoContext ctx) {
+    Type methodOwnerType = node.getType();
+    Builtins builtins = ctx.getBuiltins();
+    Type any = builtins.any();
+    Type warning = builtins.warning();
+    return methodOwnerType.isEigenType()
+            && builtins.nothing() != methodOwnerType
+            && any.getEigentype() != methodOwnerType
+            && warning.getEigentype() != methodOwnerType;
+  }
+
   @Specialization
   Object doDataflowError(
       VirtualFrame frame,

diff --git a/...runtime/src/main/java/org/enso/interpreter/node/callable/resolver/MethodResolverNode.java b/...runtime/src/main/java/org/enso/interpreter/node/callable/resolver/MethodResolverNode.java
@@ -49,7 +49,6 @@ Function resolveCached(
   }
 
   @Specialization(replaces = "resolveCached")
-  @CompilerDirectives.TruffleBoundary
   Function resolveUncached(Type self, UnresolvedSymbol symbol) {
     return symbol.resolveFor(self);
   }

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Nothing.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Nothing.java
@@ -4,6 +4,7 @@
 
 @BuiltinType(name = "Standard.Base.Nothing.Nothing")
 public class Nothing extends Builtin {
+
   @Override
   protected boolean containsValues() {
     return false;

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java
@@ -307,6 +307,10 @@ public Type getEigentype() {
     return eigentype;
   }
 
+  public boolean isEigenType() {
+    return eigentype == this;
+  }
+
   public void registerConstructor(AtomConstructor constructor) {
     constructors.put(constructor.getName(), constructor);
   }

diff --git a/test/Tests/src/Semantic/Any_Spec.enso b/test/Tests/src/Semantic/Any_Spec.enso
@@ -1,10 +1,8 @@
 from Standard.Base import all
 
-from Standard.Test import Test
+from Standard.Test import Test, Test_Suite
 import Standard.Test.Extensions
-
-type My_Type
-    Value a
+from project.Semantic.Definitions.Any_Types import all
 
 spec =
     Test.group "Any.map_nothing" <|
@@ -39,3 +37,23 @@ spec =
             (1 != 2) . should_be_true
             (1 != 1) . should_be_false
 
+    Test.group "Any's methods" <|
+        Test.specify "should not be overridable when called statically" <|
+            My_Type.Value 33 . x . should_equal "Any:(My_Type.Value 33)"
+            With_X.Value 44 . x . should_equal "With_X:(With_X.Value 44)"
+            With_Y.Value 44 . x . should_equal "With_Y:With_Y(44)"
+            Any.x . to_text . should_equal "Any.type.x[Any_Types.enso:6-26]"
+            My_Type.x . should_equal "Any:My_Type"
+            With_X.x . to_text . should_equal "Any:With_X"
+            With_X.y . to_text . should_equal "With_X.type.y[Any_Types.enso:12-32]"
+            With_Y.x . to_text . should_equal "Any:With_Y"
+            With_Y.y . to_text . should_equal "With_Y.type.y[Any_Types.enso:18-38]"
+            With_X.to_text . to_text . should_equal "With_X"
+            With_Y.to_text . to_text . should_equal "With_Y"
+            Any.x self=With_X . should_equal "Any:With_X"
+            Any.x self=With_Y . should_equal "Any:With_Y"
+            Any.x (My_Type.Value 22) . should_equal "Any:(My_Type.Value 22)"
+            Any.x (With_X.Value 22) . should_equal "Any:(With_X.Value 22)"
+            Any.x (With_Y.Value 22) . should_equal "Any:With_Y(22)"
+
+main = Test_Suite.run_main spec
diff --git a/test/Tests/src/Semantic/Definitions/Any_Types.enso b/test/Tests/src/Semantic/Definitions/Any_Types.enso
@@ -0,0 +1,20 @@
+from Standard.Base import all
+
+type My_Type
+    Value a
+
+Any.x self = "Any:" + self.to_text
+
+type With_X
+    Value b
+
+    x self = "With_X:" + self.to_text
+    y self = "With_X:" + self.to_text
+
+type With_Y
+    Value b
+
+    x self = "With_Y:" + self.to_text
+    y self = "With_Y:" + self.to_text
+
+    to_text self = "With_Y("+self.b.to_text+")"
diff --git a/...Tests/src/Semantic/Names/Definitions.enso → ...Tests/src/Semantic/Definitions/Names.enso b/...Tests/src/Semantic/Names/Definitions.enso → ...Tests/src/Semantic/Definitions/Names.enso
diff --git a/test/Tests/src/Semantic/Meta_Spec.enso b/test/Tests/src/Semantic/Meta_Spec.enso
@@ -370,4 +370,5 @@ spec =
             (.is_nothing) . is_a Function . should_equal True
             Meta.type_of (_.is_nothing) . should_equal Function
             Meta.type_of (.is_nothing) . should_equal Function
+
 main = Test_Suite.run_main spec
diff --git a/test/Tests/src/Semantic/Names_Spec.enso b/test/Tests/src/Semantic/Names_Spec.enso
@@ -1,13 +1,13 @@
 from Standard.Base import all
 
-from project.Semantic.Names.Definitions import another_method, another_constant, method_with_local_vars, Bar_Data, Bar
-import project.Semantic.Names.Definitions
+from project.Semantic.Definitions.Names import another_method, another_constant, method_with_local_vars, Bar_Data, Bar
+import project.Semantic.Definitions.Names
 
 from Standard.Test import Test
 import Standard.Test.Extensions
 
-Definitions.Foo.my_method self = case self of
-    Definitions.Foo.Value x y z -> x * y * z
+Names.Foo.my_method self = case self of
+    Names.Foo.Value x y z -> x * y * z
 
 get_foo module = module.Foo
 
@@ -18,14 +18,14 @@ add_one (x = 0) = x + 1
 spec =
     Test.group "Qualified Names" <|
         Test.specify "should allow to call constructors in a qualified manner" <|
-            Definitions.Foo.Value 1 2 3 . sum . should_equal 6
+            Names.Foo.Value 1 2 3 . sum . should_equal 6
         Test.specify "should allow pattern matching in a qualified manner" <|
-            v = Definitions.Foo.Value 1 2 3
+            v = Names.Foo.Value 1 2 3
             res = case v of
-                Definitions.Foo.Value a b c -> a + b + c
+                Names.Foo.Value a b c -> a + b + c
             res.should_equal 6
         Test.specify "should allow defining methods on qualified names" <|
-            v = Definitions.Foo.Value 2 3 5
+            v = Names.Foo.Value 2 3 5
             v.my_method.should_equal 30
     Test.group "Lowercase Methods" <|
         Test.specify "should allow calling methods without a target" <|
@@ -38,8 +38,8 @@ spec =
             another_method 10 . should_equal 10
             another_constant . should_equal 10
         Test.specify "should allow calling methods with fully qualified module name" <|
-            (Definitions.another_method 10).should_equal 10
-            v = Definitions.another_method
+            (Names.another_method 10).should_equal 10
+            v = Names.another_method
             v 10 . should_equal 10
         Test.specify "should be resolved correctly in the presence of variables with the same name" <|
             method_with_local_vars 1 . should_equal 13