expressionUpdates do not contain Method Pointer of operators (#7659)

close #7520 Changelog: - update: SectionsToBinOp compiler pass produces function application for left sections - refactor: simplify the registration of builtin methods
2024-11-22 11:52:59 +03:00 · 2023-09-15 09:01:21 +01:00 · 2023-09-15 09:01:21 +01:00 · bbf96f0d16
commit bbf96f0d16
parent 5eb4944ccf
8 changed files with 191 additions and 75 deletions
--- a/engine/runtime-instrument-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
+++ b/engine/runtime-instrument-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
@ -15,12 +15,14 @@ import com.oracle.truffle.api.nodes.Node;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.UUID;
-
+import java.util.function.Consumer;
 import org.enso.interpreter.instrument.profiling.ExecutionTime;
 import org.enso.interpreter.instrument.profiling.ProfilingInfo;
+import org.enso.interpreter.node.ClosureRootNode;
 import org.enso.interpreter.node.ExpressionNode;
 import org.enso.interpreter.node.callable.FunctionCallInstrumentationNode;
 import org.enso.interpreter.node.expression.builtin.meta.TypeOfNode;
+import org.enso.interpreter.runtime.Module;
 import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
 import org.enso.interpreter.runtime.callable.function.Function;
 import org.enso.interpreter.runtime.control.TailCallException;
@ -29,13 +31,9 @@ import org.enso.interpreter.runtime.error.DataflowError;
 import org.enso.interpreter.runtime.error.PanicException;
 import org.enso.interpreter.runtime.error.PanicSentinel;
 import org.enso.interpreter.runtime.state.State;
+import org.enso.interpreter.runtime.tag.AvoidIdInstrumentationTag;
 import org.enso.interpreter.runtime.tag.IdentifiedTag;
 import org.enso.interpreter.runtime.type.Constants;
-import org.enso.interpreter.runtime.Module;
-
-import java.util.function.Consumer;
-import org.enso.interpreter.node.ClosureRootNode;
-import org.enso.interpreter.runtime.tag.AvoidIdInstrumentationTag;

 /** An instrument for getting values from AST-identified expressions. */
@TruffleInstrument.Registration(
@ -56,7 +54,7 @@ public class IdExecutionInstrument extends TruffleInstrument implements IdExecut
    this.env = env;
  }

-  /** Factory for creating new id event nodes **/
+  /** Factory for creating new id event nodes. */
  private static class IdEventNodeFactory implements ExecutionEventNodeFactory {

      private final CallTarget entryCallTarget;
--- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala
+++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala
@ -806,6 +806,143 @@ class RuntimeServerTest
    )
  }

+  it should "send method pointer updates of builtin operators" in {
+    val contextId  = UUID.randomUUID()
+    val requestId  = UUID.randomUUID()
+    val moduleName = "Enso_Test.Test.Main"
+
+    val metadata = new Metadata
+    val id_x_1   = metadata.addItem(48, 5, "aa")
+
+    val code =
+      """from Standard.Base import all
+        |
+        |main =
+        |    x_1 = 3 ^ 4
+        |    x_1
+        |""".stripMargin.linesIterator.mkString("\n")
+    val contents = metadata.appendToCode(code)
+    val mainFile = context.writeMain(contents)
+
+    // create context
+    context.send(Api.Request(requestId, Api.CreateContextRequest(contextId)))
+    context.receive shouldEqual Some(
+      Api.Response(requestId, Api.CreateContextResponse(contextId))
+    )
+
+    // open file
+    context.send(
+      Api.Request(Api.OpenFileNotification(mainFile, contents))
+    )
+    context.receiveNone shouldEqual None
+
+    // push main
+    context.send(
+      Api.Request(
+        requestId,
+        Api.PushContextRequest(
+          contextId,
+          Api.StackItem.ExplicitCall(
+            Api.MethodPointer(moduleName, moduleName, "main"),
+            None,
+            Vector()
+          )
+        )
+      )
+    )
+    context.receiveNIgnoreStdLib(4) should contain theSameElementsAs Seq(
+      Api.Response(Api.BackgroundJobsStartedNotification()),
+      Api.Response(requestId, Api.PushContextResponse(contextId)),
+      TestMessages.update(
+        contextId,
+        id_x_1,
+        ConstantsGen.INTEGER,
+        Api.MethodCall(
+          Api.MethodPointer(
+            "Standard.Base.Data.Numbers",
+            "Standard.Base.Data.Numbers.Integer",
+            "^"
+          )
+        )
+      ),
+      context.executionComplete(contextId)
+    )
+  }
+
+  it should "send method pointer updates of partially applied builtin operators" in {
+    val contextId  = UUID.randomUUID()
+    val requestId  = UUID.randomUUID()
+    val moduleName = "Enso_Test.Test.Main"
+
+    val metadata = new Metadata
+    val id_x_1   = metadata.addItem(48, 5, "aa")
+    val id_x_2   = metadata.addItem(64, 7, "ab")
+
+    val code =
+      """from Standard.Base import all
+        |
+        |main =
+        |    x_1 = "4" +
+        |    x_2 = x_1 "2"
+        |    x_2
+        |""".stripMargin.linesIterator.mkString("\n")
+    val contents = metadata.appendToCode(code)
+    val mainFile = context.writeMain(contents)
+
+    // create context
+    context.send(Api.Request(requestId, Api.CreateContextRequest(contextId)))
+    context.receive shouldEqual Some(
+      Api.Response(requestId, Api.CreateContextResponse(contextId))
+    )
+
+    // open file
+    context.send(
+      Api.Request(Api.OpenFileNotification(mainFile, contents))
+    )
+    context.receiveNone shouldEqual None
+
+    // push main
+    context.send(
+      Api.Request(
+        requestId,
+        Api.PushContextRequest(
+          contextId,
+          Api.StackItem.ExplicitCall(
+            Api.MethodPointer(moduleName, moduleName, "main"),
+            None,
+            Vector()
+          )
+        )
+      )
+    )
+    val textPlusMethodPointer = Api.MethodPointer(
+      "Standard.Base.Data.Text",
+      "Standard.Base.Data.Text.Text",
+      "+"
+    )
+    context.receiveNIgnoreStdLib(5) should contain theSameElementsAs Seq(
+      Api.Response(Api.BackgroundJobsStartedNotification()),
+      Api.Response(requestId, Api.PushContextResponse(contextId)),
+      TestMessages.update(
+        contextId,
+        id_x_1,
+        ConstantsGen.FUNCTION,
+        methodCall = Some(Api.MethodCall(textPlusMethodPointer, Vector(1))),
+        payload = Api.ExpressionUpdate.Payload.Value(
+          None,
+          Some(Api.FunctionSchema(textPlusMethodPointer, Vector(1)))
+        )
+      ),
+      TestMessages.update(
+        contextId,
+        id_x_2,
+        ConstantsGen.TEXT,
+        Api.MethodCall(textPlusMethodPointer)
+      ),
+      context.executionComplete(contextId)
+    )
+  }
+
  it should "send method pointer updates of partially applied constructors" in {
    val contextId  = UUID.randomUUID()
    val requestId  = UUID.randomUUID()
--- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Builtins.java
+++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Builtins.java
@ -1,5 +1,6 @@
 package org.enso.interpreter.runtime.builtin;

+import com.oracle.truffle.api.CompilerDirectives;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
@ -16,7 +17,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
-
 import org.enso.compiler.Passes;
 import org.enso.compiler.context.CompilerContext;
 import org.enso.compiler.context.FreshNameSupply;
@ -47,16 +47,11 @@ import org.enso.interpreter.node.expression.builtin.runtime.Context;
 import org.enso.interpreter.node.expression.builtin.text.Text;
 import org.enso.interpreter.runtime.EnsoContext;
 import org.enso.interpreter.runtime.Module;
-import org.enso.interpreter.runtime.builtin.Error;
-import org.enso.interpreter.runtime.builtin.Number;
-import org.enso.interpreter.runtime.builtin.System;
 import org.enso.interpreter.runtime.callable.function.Function;
 import org.enso.interpreter.runtime.data.Type;
 import org.enso.interpreter.runtime.scope.ModuleScope;
 import org.enso.pkg.QualifiedName;

-import com.oracle.truffle.api.CompilerDirectives;
-
 /** Container class for static predefined atoms, methods, and their containing scope. */
 public final class Builtins {

@ -187,12 +182,11 @@ public final class Builtins {
        // Register a builtin method iff it is marked as auto-register.
        // Methods can only register under a type or, if we deal with a static method, it's eigen-type.
        // Such builtins are available on certain types without importing the whole stdlib, e.g. Any or Number.
-        methods.entrySet().stream().forEach(entry -> {
-          Type tpe = entry.getValue().isAutoRegister ? (!entry.getValue().isStatic() ? type : type.getEigentype()) : null;
+        methods.forEach((key, value) -> {
+          Type tpe = value.isAutoRegister ? (!value.isStatic() ? type : type.getEigentype()) : null;
          if (tpe != null) {
-            LoadedBuiltinMethod value = entry.getValue();
            Optional<BuiltinFunction> fun = value.toFunction(language, false);
-            fun.ifPresent(f -> scope.registerMethod(tpe, entry.getKey(), f.getFunction()));
+            fun.ifPresent(f -> scope.registerMethod(tpe, key, f.getFunction()));
          }
        });
      }
@ -235,7 +229,6 @@ public final class Builtins {
   * builtin type per row. The format of the row is as follows: <Enso name of the builtin
   * type>:<Name of the class representing it>:[<field1>,<field2>,...] where the last column gives a
   * list of optional type's fields.
-   *
   */
  private static List<Constructor<? extends Builtin>> readBuiltinTypes() {
    ClassLoader classLoader = Builtins.class.getClassLoader();
--- a/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/SectionsToBinOp.scala
+++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/desugar/SectionsToBinOp.scala
@ -95,8 +95,7 @@ case object SectionsToBinOp extends IRPass {
    * @param section the section to desugar
    * @return the result of desugaring `section`
    */
-  //noinspection DuplicatedCode
-  def desugarSections(
+  private def desugarSections(
    section: Section,
    freshNameSupply: FreshNameSupply
  ): Expression = {
@ -146,21 +145,16 @@ case object SectionsToBinOp extends IRPass {
          )

        } else {
-          val opCall = Application.Prefix(
+          Application.Prefix(
            function             = op,
-            arguments            = List(arg, rightCallArg),
+            arguments            = List(arg),
            hasDefaultsSuspended = false,
-            location             = None,
+            location             = loc,
            passData,
            diagnostics
          )
-
-          Function.Lambda(
-            List(rightDefArg),
-            opCall,
-            loc
-          )
        }
+
      case Section.Sides(op, loc, passData, diagnostics) =>
        val leftArgName = freshNameSupply.newName()
        val leftCallArg =
--- a/engine/runtime/src/test/java/org/enso/compiler/CompilerTest.java
+++ b/engine/runtime/src/test/java/org/enso/compiler/CompilerTest.java
@ -1,5 +1,8 @@
 package org.enso.compiler;

+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
 import com.oracle.truffle.api.source.Source;
 import java.io.File;
 import java.io.IOException;
@ -10,8 +13,6 @@ import org.enso.compiler.core.EnsoParser;
 import org.enso.compiler.core.IR;
 import org.enso.compiler.core.ir.Module;
 import org.junit.AfterClass;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
 import org.junit.BeforeClass;

 public abstract class CompilerTest {
--- a/engine/runtime/src/test/scala/org/enso/compiler/test/pass/desugar/SectionsToBinOpTest.scala
+++ b/engine/runtime/src/test/scala/org/enso/compiler/test/pass/desugar/SectionsToBinOpTest.scala
@ -7,6 +7,7 @@ import org.enso.compiler.core.ir.{
  DefinitionArgument,
  Expression,
  Function,
+  Literal,
  Name
 }
 import org.enso.compiler.core.ir.expression.Application
@ -63,26 +64,20 @@ class SectionsToBinOpTest extends CompilerTest {
          |(1 +)
          |""".stripMargin.preprocessExpression.get.desugar

-      ir shouldBe an[Function.Lambda]
+      ir shouldBe an[Application.Prefix]
      ir.location shouldBe defined

-      val irLam = ir.asInstanceOf[Function.Lambda]
-      irLam.arguments.length shouldEqual 1
+      val irApp = ir.asInstanceOf[Application.Prefix]
+      irApp.arguments.length shouldEqual 1
+      irApp.arguments.head shouldBe an[CallArgument.Specified]

-      val lamArgName =
-        irLam.arguments.head.asInstanceOf[DefinitionArgument.Specified].name
+      val irAppArgument =
+        irApp.arguments.head.asInstanceOf[CallArgument.Specified]
+      irAppArgument.value shouldBe an[Literal.Number]

-      val lamBody = irLam.body.asInstanceOf[Application.Prefix]
-      lamBody.arguments.length shouldEqual 2
-      val lamBodyFirstArg =
-        lamBody
-          .arguments(1)
-          .asInstanceOf[CallArgument.Specified]
-          .value
-          .asInstanceOf[Name.Literal]
-
-      lamBodyFirstArg.name shouldEqual lamArgName.name
-      lamBodyFirstArg.getId should not equal lamArgName.getId
+      irApp.function shouldBe an[Name.Literal]
+      val irAppFunction = irApp.function.asInstanceOf[Name.Literal]
+      irAppFunction.name shouldEqual "+"
    }

    "work for sides sections" in {
@ -94,8 +89,7 @@ class SectionsToBinOpTest extends CompilerTest {
          |""".stripMargin.preprocessExpression.get.desugar

      ir shouldBe an[Function.Lambda]
-      // TODO[DB] Section.Sides location is not parsed
-      //ir.location shouldBe defined
+      ir.location shouldBe defined

      val leftLam = ir.asInstanceOf[Function.Lambda]
      leftLam.arguments.length shouldEqual 1
@ -167,14 +161,12 @@ class SectionsToBinOpTest extends CompilerTest {
          .asInstanceOf[Function.Lambda]

      ir.body
-        .asInstanceOf[Function.Lambda]
-        .body shouldBe an[Application.Prefix]
+        .asInstanceOf[Application.Prefix]
+        .function shouldBe an[Name.Literal]
      ir.body
-        .asInstanceOf[Function.Lambda]
-        .body
        .asInstanceOf[Application.Prefix]
        .arguments
-        .length shouldEqual 2
+        .length shouldEqual 1
    }

    "flip the arguments when a right section's argument is a blank" in {
--- a/test/Tests/src/Data/Text/Regex_Spec.enso
+++ b/test/Tests/src/Data/Text/Regex_Spec.enso
@ -296,32 +296,32 @@ spec =
            pattern = Regex.compile "(?<건반>[a-z]+)"
            pattern.replace "foo bar, baz" "[$<건반>]" . should_equal "[foo] [bar], [baz]"

-         Text.group "should correctly evaluate documentation examples" <|
-            Test.specify "example 1" <|
-                pattern = Regex.compile 'aa'
-                pattern.replace 'aaa' 'b' . should_equal 'ba'
+        Test.group "should correctly evaluate documentation examples" <|
+           Test.specify "example 1" <|
+               pattern = Regex.compile 'aa'
+               pattern.replace 'aaa' 'b' . should_equal 'ba'

-            Test.specify "example 2" <|
-                pattern = Regex.compile '[lo]'
-                pattern.replace 'Hello World!' '#' . should_equal 'He### W#r#d!'
+           Test.specify "example 2" <|
+               pattern = Regex.compile '[lo]'
+               pattern.replace 'Hello World!' '#' . should_equal 'He### W#r#d!'

-            Test.specify "example 3" <|
-                pattern = Regex.compile 'l'
-                pattern.replace 'Hello World!' '#' only_first=True . should_equal 'He#lo World!'
+           Test.specify "example 3" <|
+               pattern = Regex.compile 'l'
+               pattern.replace 'Hello World!' '#' only_first=True . should_equal 'He#lo World!'

-            Test.specify "example 4" <|
-                pattern = Regex.compile '"(.*?)"'
-                pattern.replace '"abc" foo "bar" baz' '($1)' . should_equal '(abc) foo (bar) baz'
+           Test.specify "example 4" <|
+               pattern = Regex.compile '"(.*?)"'
+               pattern.replace '"abc" foo "bar" baz' '($1)' . should_equal '(abc) foo (bar) baz'

-            Test.specify "example 5" <|
-                pattern = Regex.compile "aa"
-                input = "aa ab aa ac ad aa aa ax"
-                match = pattern.replace input "xyz"
-                match . should_equal "xyz ab xyz ac ad xyz xyz ax"
+           Test.specify "example 5" <|
+               pattern = Regex.compile "aa"
+               input = "aa ab aa ac ad aa aa ax"
+               match = pattern.replace input "xyz"
+               match . should_equal "xyz ab xyz ac ad xyz xyz ax"

-            Test.specify "example 6" <|
-                pattern = Regex.compile "([a-z]+)"
-                pattern.replace "foo bar, baz" "[$1]" . should_equal "[foo] [bar], [baz]"
+           Test.specify "example 6" <|
+               pattern = Regex.compile "([a-z]+)"
+               pattern.replace "foo bar, baz" "[$1]" . should_equal "[foo] [bar], [baz]"

        Test.specify "`replace` with an empty pattern should be an error" <|
            pattern = Regex.compile ""
--- a/test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
+++ b/test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
@ -3,3 +3,4 @@ type Number

@Builtin_Type
 type Integer
+    ^ self that = @Builtin_Method "Integer.^"