Instrumentor allows executing expressions in local context #8293

4e6 · 2023-11-13T16:01:39Z

Extracted from the discussion comment #8148 (comment)

The onReturn method of the Instrumentor should contain an extra expression argument

    on_return self (fn : Text -> Any -> Nothing) (expression : Text | Nothing = Nothing)

that will be evaluated in the context of the intercepted node.

The text was updated successfully, but these errors were encountered:

JaroslavTulach · 2023-11-19T09:20:03Z

Direct & naive implementation is ready:

diff --git distribution/lib/Standard/Base/0.0.0-dev/src/Meta.enso distribution/lib/Standard/Base/0.0.0-dev/src/Meta.enso
index b8cdf46d29..feca506f80 100644
--- distribution/lib/Standard/Base/0.0.0-dev/src/Meta.enso
+++ distribution/lib/Standard/Base/0.0.0-dev/src/Meta.enso
@@ -622,8 +622,10 @@ type Instrumentor
 
        Arguments:
        - fn: The callback function accepting UUID and computed value
-    on_return self (fn : Text -> Any -> Nothing) =
-        new = instrumentor_builtin "onReturn" [ self.impl, fn ]
+       - expression: Expression to evaluate on_return - by default Nothing -
+         e.g. to provide the return value of the function
+    on_return self (fn : Text -> Any -> Nothing) (expression : Text|Nothing)=Nothing =
+        new = instrumentor_builtin "onReturn" [ self.impl, fn, expression ]
         Instrumentor.Value new
 
     ## PRIVATE
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/Instrumentor.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/Instrumentor.java
index 007a9e1d75..6a00c4f100 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/Instrumentor.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/Instrumentor.java
@@ -21,6 +21,7 @@ final class Instrumentor implements EnsoObject, IdExecutionService.Callbacks {
   private final Module module;
   private final Object onEnter;
   private final Object onReturn;
+  private final Object onReturnExpr;
   private final Object onCall;
   private final EventBinding<?> handle;
 
@@ -30,16 +31,18 @@ final class Instrumentor implements EnsoObject, IdExecutionService.Callbacks {
     this.target = target;
     this.onEnter = null;
     this.onReturn = null;
+    this.onReturnExpr = null;
     this.onCall = null;
     this.handle = null;
   }
 
-  Instrumentor(Instrumentor orig, Object onEnter, Object onReturn, Object onCall, boolean activate) {
+  Instrumentor(Instrumentor orig, Object onEnter, Object onReturn, Object onReturnExpr, Object onCall, boolean activate) {
     this.module = orig.module;
     this.service = orig.service;
     this.target = orig.target;
     this.onEnter = onEnter != null ? onEnter : orig.onEnter;
     this.onReturn = onReturn != null ? onReturn : orig.onReturn;
+    this.onReturnExpr = onReturnExpr != null ? onReturnExpr : orig.onReturnExpr;
     this.onCall = onCall != null ? onCall : orig.onCall;
     this.handle = !activate ? null : service.bind(
       module, target, this, new Timer.Disabled()
@@ -73,7 +76,9 @@ final class Instrumentor implements EnsoObject, IdExecutionService.Callbacks {
   public void updateCachedResult(IdExecutionService.Info info) {
     try {
       if (onReturn != null) {
-        InteropLibrary.getUncached().execute(onReturn, info.getId().toString(), info.getResult());
+        var iop = InteropLibrary.getUncached();
+        var result = onReturnExpr == null || iop.isNull(onReturnExpr) ? info.getResult() : info.eval(iop.asString(onReturnExpr));
+        iop.execute(onReturn, info.getId().toString(), result);
       }
     } catch (InteropException ignored) {
     }
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/InstrumentorBuiltin.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/InstrumentorBuiltin.java
index a9681276da..fe45e841d5 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/InstrumentorBuiltin.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/InstrumentorBuiltin.java
@@ -1,20 +1,18 @@
 package org.enso.interpreter.node.expression.builtin.meta;
 
 
-import com.oracle.truffle.api.nodes.Node;
-
 import org.enso.interpreter.dsl.BuiltinMethod;
 import org.enso.interpreter.runtime.EnsoContext;
 import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
 import org.enso.interpreter.runtime.callable.function.Function;
 import org.enso.interpreter.runtime.data.text.Text;
 import org.enso.interpreter.runtime.data.vector.ArrayLikeAtNode;
-import org.enso.interpreter.runtime.data.vector.ArrayLikeCoerceToArrayNode;
 import org.enso.interpreter.runtime.data.vector.ArrayLikeLengthNode;
 import org.enso.interpreter.runtime.error.PanicException;
 
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.interop.InvalidArrayIndexException;
+import com.oracle.truffle.api.nodes.Node;
 
 @BuiltinMethod(
     type = "Meta",
@@ -33,7 +31,7 @@ public class InstrumentorBuiltin extends Node {
       if (atNode.executeAt(args, 0) instanceof Instrumentor b) {
         ret = switch (op) {
           case "onEnter" -> onEnter(b, atNode.executeAt(args, 1));
-          case "onReturn" -> onReturn(b, atNode.executeAt(args, 1));
+          case "onReturn" -> onReturn(b, atNode.executeAt(args, 1), atNode.executeAt(args, 2));
           case "onCall" -> onCall(b, atNode.executeAt(args, 1));
           case "activate" -> activate(b, atNode.executeAt(args, 1));
           case "deactivate" -> b.deactivate();
@@ -70,16 +68,16 @@ public class InstrumentorBuiltin extends Node {
   @CompilerDirectives.TruffleBoundary
   private Object onEnter(Instrumentor b, Object arg) {
     if (arg instanceof Function fn) {
-      return new Instrumentor(b, fn, null, null, false);
+      return new Instrumentor(b, fn, null, null, null, false);
     } else {
       return null;
     }
   }
 
   @CompilerDirectives.TruffleBoundary
-  private Object onReturn(Instrumentor b, Object arg) {
+  private Object onReturn(Instrumentor b, Object arg, Object expr) {
     if (arg instanceof Function fn) {
-      return new Instrumentor(b, null, fn, null, false);
+      return new Instrumentor(b, null, fn, expr, null, false);
     } else {
       return null;
     }
@@ -88,7 +86,7 @@ public class InstrumentorBuiltin extends Node {
   @CompilerDirectives.TruffleBoundary
   private Object onCall(Instrumentor b, Object arg) {
     if (arg instanceof Function fn) {
-      return new Instrumentor(b, null, null, fn, false);
+      return new Instrumentor(b, null, null, null, fn, false);
     } else {
       return null;
     }
@@ -97,7 +95,7 @@ public class InstrumentorBuiltin extends Node {
   @CompilerDirectives.TruffleBoundary
   private Object activate(Instrumentor b, Object arg) {
     if (arg instanceof Function fn) {
-      var builder = new Instrumentor(b, null, null, null, true);
+      var builder = new Instrumentor(b, null, null, null, null, true);
       var ctx = EnsoContext.get(this);
       return ctx.getResourceManager().register(builder, fn);
     } else {
diff --git test/Tests/src/Semantic/Instrumentor_Spec.enso test/Tests/src/Semantic/Instrumentor_Spec.enso
index 4e130f4fad..1dcdeff9fd 100644
--- test/Tests/src/Semantic/Instrumentor_Spec.enso
+++ test/Tests/src/Semantic/Instrumentor_Spec.enso
@@ -44,6 +44,36 @@ spec =
             # no more instrumenting after finalize
             b.to_vector.length . should_equal 1
 
+        Test.specify "access local variables " <|
+            b = Vector.new_builder
+
+            collect uuid:Text result =
+                a_plus_b_uuid = "00000000-aaaa-bbbb-0000-000000000000" # UUID for a+b
+                if uuid == a_plus_b_uuid then
+                    b.append result
+                Nothing
+
+            instrumenter = Meta.meta .fib . instrument . on_return collect expression="b-a" . activate
+
+            instrumenter . with _->
+                result = fib 10
+
+                v = b.to_vector
+
+                v.length . should_equal 1
+                v.at 0 . should_equal 89
+                result . should_equal 89
+
+            instrumenter.finalize
+            # no op:
+            instrumenter.finalize
+
+            result = fib 10
+            result . should_equal 89
+
+            # no more instrumenting after finalize
+            b.to_vector.length . should_equal 1
+
         Test.specify "replay with caches and specify different result" <|
             replay uuid:Text = case uuid of
                 "00000000-ffff-bbbb-0000-000000000000" -> 42

JaroslavTulach · 2023-11-19T09:21:56Z

However the previous implementation has a problem! It tries to evaluate expression at all locations in the fib function and not all of them have access to a and b variable. Ideally we'd evaluate the expression on at "00000000-aaaa-bbbb-0000-000000000000" # UUID for a+b and nowhere else.

enso-bot · 2023-11-20T05:45:04Z

Jaroslav Tulach reports a new STANDUP for yesterday (2023-11-19):

Progress: - #8293 (comment)

merged caches: Store IR caches in suitable format to read them "on demand" #8207
reviews & emails: Backend self sampling #8309 (review)
Sunday: It should be finished by 2023-11-22.

Next Day: Bigfixing & stabilization

enso-bot · 2023-11-21T05:34:36Z

Jaroslav Tulach reports a new STANDUP for yesterday (2023-11-20):

Progress: - suspended thunk execution & merge of #8331

Unary operator without RHS crashes with NPE when translating to IR #8332 fixed by Syntax error on unexpected unary minus #8333
Y-WebSocket server: JaroslavTulach/graaljs-websocket@9bdd7df
fighting with webpack & polyfil
asking about Graal.js imports: https://graalvm.slack.com/archives/CN9L59P61/p1700499196140779?thread_ts=1700499044.997459&cid=CN9L59P61
standups, meetings, demos It should be finished by 2023-11-22.

Next Day: Bugfixing whatever gets planned

4e6 added p-low Low priority -compiler -libs Libraries: New libraries to be implemented labels Nov 13, 2023

4e6 assigned 4e6 and JaroslavTulach Nov 13, 2023

github-project-automation bot added this to Issues Board Nov 13, 2023

github-project-automation bot moved this to ❓New in Issues Board Nov 13, 2023

4e6 mentioned this issue Nov 13, 2023

Inline Execution #8148

Merged

3 tasks

jdunkerley unassigned 4e6 Nov 14, 2023

jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Nov 14, 2023

JaroslavTulach moved this from 📤 Backlog to ⚙️ Design in Issues Board Nov 19, 2023

JaroslavTulach moved this from ⚙️ Design to 👁️ Code review in Issues Board Nov 20, 2023

JaroslavTulach mentioned this issue Nov 20, 2023

Specify expression to get more advanced results on_return callback #8331

Merged

3 tasks

JaroslavTulach closed this as completed in #8331 Nov 20, 2023

github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Nov 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Instrumentor allows executing expressions in local context #8293

Instrumentor allows executing expressions in local context #8293

4e6 commented Nov 13, 2023

JaroslavTulach commented Nov 19, 2023

JaroslavTulach commented Nov 19, 2023

enso-bot bot commented Nov 20, 2023

enso-bot bot commented Nov 21, 2023

Instrumentor allows executing expressions in local context #8293

Instrumentor allows executing expressions in local context #8293

Comments

4e6 commented Nov 13, 2023

JaroslavTulach commented Nov 19, 2023

JaroslavTulach commented Nov 19, 2023

enso-bot bot commented Nov 20, 2023

enso-bot bot commented Nov 21, 2023