Inline Execution

[4e6]

Pull Request Description

close #8132

Update the executionContext/executeExpression request to execute expressions in the local scope.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

• The documentation has been updated, if necessary.
• All code follows the
  Scala,
  Java,
  and
  Rust
  style guides. In case you are using a language not listed above, follow the Rust style guide.
• All code has been tested:
  • Unit tests have been written where possible.

[JaroslavTulach]

According to Marcin, this is what we should do...

[JaroslavTulach]

Doing flatMap and then Some is strange. map is better.

[hubertp]

The current state looks OK to me - the changes seems mostly cosmetic. I will leave the final word to @JaroslavTulach

[JaroslavTulach]

@4e6 spent quite a long time to explain me that there are multiple visualizations hidden behind this single bind. With such information it is clear that my original API design proposal cannot work. Here is the modified one:

diff --git engine/polyglot-api/src/main/java/org/enso/polyglot/debugger/IdExecutionService.java engine/polyglot-api/src/main/java/org/enso/polyglot/debugger/IdExecutionService.java
index fc8e712796..eac7025a08 100644
--- engine/polyglot-api/src/main/java/org/enso/polyglot/debugger/IdExecutionService.java
+++ engine/polyglot-api/src/main/java/org/enso/polyglot/debugger/IdExecutionService.java
@@ -9,37 +9,56 @@ import java.util.UUID;
 public interface IdExecutionService {
   String INSTRUMENT_ID = "id-value-extractor";
 
+  public abstract class Info {
+    protected Info() {
+      assert getClass().getName().equals("org.enso....InfoImpl");
+    }
+
+    /** @return UUID of the node, never {@code null} */
+    public abstract UUID getUuid();
+    /** @return associated result or {@code null} if there is no associated result */
+    public abstract Object getResult();
+
+    /** @return {@code true} when the result is panic, {@code false} otherwise */
+    public abstract boolean isPanic();
+
+    /** @return time (in nanoseconds) needed to compute the result or {@code -1} when not available */
+    public abstract long getElapsedTime();
+
+    /** Evaluates given code in the context of current UUID location.
+     * @param code the Enso code to evaluate
+     * @return result of the evaluation
+     */
+    public abstract Object eval(String code);
+  }
+
   public interface Callbacks {
 
     /**
      * Finds out previously computed result for given id. If a result is returned, then the
      * execution of given node is skipped and the value is returned back.
      *
-     * @param nodeId identification of the node to be computed
+     * @param info info with UUID the node to be computed
      * @return {@code null} should the execution of the node be performed; any other value to skip
      *     the execution and return the value as a result.
      */
-    Object findCachedResult(Object materializedFrame, Object node, UUID nodeId);
+    Object findCachedResult(Info info);
 
     /**
      * Notifies when an execution of a node is over.
      *
-     * @param nodeId expression node id that has been computed
-     * @param result the just computed result
-     * @param isPanic was the result a panic?
-     * @param nanoElapsedTime how long it took to compute the result?
+     * @param info info with node id, {@link Info#getResult()}, {@link Info#isPanic()} and {@link Info#getElapsedTime()}
      */
-    void updateCachedResult(UUID nodeId, Object result, boolean isPanic, long nanoElapsedTime);
+    void updateCachedResult(Info info);
 
     /**
      * Notification when a returned value is a function.
      *
-     * @param nodeId identification of the node to be computed
-     * @param result info about function call
+     * @param info with identification of the node and {@link Info#getResult() info about function call}
      * @return {@code null} should the execution of the node be performed; any other value to skip
      *     the execution and return the value as a result.
      */
-    Object onFunctionReturn(UUID nodeId, TruffleObject result);
+    Object onFunctionReturn(Info info);
   }
 
   /**

The new suggestion adds Info.eval(String) method which can be used by visualization to evaluate any code at given UUID location. Each visualization with associated expression can call this method with its own code to evaluate. The implementation inserts EvalNode into IdExecutionEventNode and uses it to evaluate the code.

There is a new Info class - an implementation of API vs. SPI pattern which is going to be implemented by IdExecutionInstrument and will have a reference to appropriate IdExecutionEventNode. Having an Info class there shall help us with future evolution of additional arguments - shall more of them be needed.

[JaroslavTulach]

• I am concerned about "speed" of eval at current state. Please measure.
• I'd like to keep consistency of Meta.Instrumentor - add the same functionality there
• otherwise OK, approving.

[JaroslavTulach]

Suggested change

	- You can execute an expression in the context of a function body. In this case,
	- You can execute an expression in the context of 'main' function body. In this case,

[JaroslavTulach]

Suggested change

	the `expressionId` should point to the body of a function. E.g. in the context
	the `expressionId` should point to the body of the `main` function. Then the available symbols

[JaroslavTulach]

Suggested change

	of `main` available symbols are `operator1`, `operator2` and `fun1`.
	are `operator1`, `operator2` and `fun1`.

[JaroslavTulach]

Thank you, @4e6, for trying to use this design with Info.

[JaroslavTulach]

Here is the usage of Info.eval, OK.