diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d79c1dab6..66aa4553a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -651,6 +651,7 @@ - [Use SHA-1 for calculating hashes of modules' IR and bindings][5791] - [Don't install Python component on Windows][5900] - [Detect potential name conflicts between exported types and FQNs][5966] +- [Ensure calls involving warnings remain instrumented][6067] [3227]: https://github.com/enso-org/enso/pull/3227 [3248]: https://github.com/enso-org/enso/pull/3248 @@ -753,6 +754,7 @@ [5791]: https://github.com/enso-org/enso/pull/5791 [5900]: https://github.com/enso-org/enso/pull/5900 [5966]: https://github.com/enso-org/enso/pull/5966 +[6067]: https://github.com/enso-org/enso/pull/6067 # Enso 2.0.0-alpha.18 (2021-10-12) diff --git 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 index 7d6bfd8c85..0b14a5f0c2 100644 --- 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 @@ -230,9 +230,9 @@ public class IdExecutionInstrument extends TruffleInstrument implements IdExecut Node node = context.getInstrumentedNode(); if (node instanceof FunctionCallInstrumentationNode - && result instanceof FunctionCallInstrumentationNode.FunctionCall) { + && result instanceof FunctionCallInstrumentationNode.FunctionCall functionCall) { UUID nodeId = ((FunctionCallInstrumentationNode) node).getId(); - onFunctionReturn(nodeId, result, context); + onFunctionReturn(nodeId, functionCall, context); } else if (node instanceof ExpressionNode) { onExpressionReturn(result, node, context); } @@ -307,11 +307,10 @@ public class IdExecutionInstrument extends TruffleInstrument implements IdExecut } @CompilerDirectives.TruffleBoundary - private void onFunctionReturn(UUID nodeId, Object result, EventContext context) throws ThreadDeath { + private void onFunctionReturn(UUID nodeId, FunctionCallInstrumentationNode.FunctionCall result, EventContext context) throws ThreadDeath { calls.put( - nodeId, new FunctionCallInfo((FunctionCallInstrumentationNode.FunctionCall) result)); - functionCallCallback.accept( - new ExpressionCall(nodeId, (FunctionCallInstrumentationNode.FunctionCall) result)); + nodeId, new FunctionCallInfo(result)); + functionCallCallback.accept(new ExpressionCall(nodeId, result)); // Return cached value after capturing the enterable function call in `functionCallCallback` Object cachedResult = cache.get(nodeId); if (cachedResult != null) { diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/NodeCountingTestInstrument.java b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/NodeCountingTestInstrument.java index b52097dbf9..cd33a79446 100644 --- a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/NodeCountingTestInstrument.java +++ b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/NodeCountingTestInstrument.java @@ -1,14 +1,22 @@ package org.enso.interpreter.test; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.EventContext; import com.oracle.truffle.api.instrumentation.ExecutionEventNode; import com.oracle.truffle.api.instrumentation.ExecutionEventNodeFactory; import com.oracle.truffle.api.instrumentation.SourceSectionFilter; import com.oracle.truffle.api.instrumentation.TruffleInstrument; import com.oracle.truffle.api.nodes.Node; +import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.api.source.SourceSection; +import org.enso.interpreter.node.MethodRootNode; +import org.enso.interpreter.node.callable.FunctionCallInstrumentationNode; +import org.enso.pkg.QualifiedName; + import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Function; @@ -22,6 +30,8 @@ public class NodeCountingTestInstrument extends TruffleInstrument { public static final String INSTRUMENT_ID = "node-count-test"; private Map all = new ConcurrentHashMap<>(); private Map> counter = new ConcurrentHashMap<>(); + + private Map calls = new ConcurrentHashMap<>(); private Env env; @Override @@ -33,7 +43,17 @@ public class NodeCountingTestInstrument extends TruffleInstrument { public void enable() { this.env .getInstrumenter() - .attachExecutionEventFactory(SourceSectionFilter.ANY, new CountingFactory()); + .attachExecutionEventFactory(SourceSectionFilter.ANY, new CountingAndFunctionCallFactory()); + } + + public void enable(SourceSectionFilter filter) { + this.env + .getInstrumenter() + .attachExecutionEventFactory(filter, new CountingAndFunctionCallFactory()); + } + + public Map registeredCalls() { + return calls; } public Map> assertNewNodes(String msg, int min, int max) { @@ -73,7 +93,7 @@ public class NodeCountingTestInstrument extends TruffleInstrument { } } - private final class CountingFactory implements ExecutionEventNodeFactory { + private final class CountingAndFunctionCallFactory implements ExecutionEventNodeFactory { @Override public ExecutionEventNode create(EventContext context) { final Node node = context.getInstrumentedNode(); @@ -81,8 +101,92 @@ public class NodeCountingTestInstrument extends TruffleInstrument { if (all.put(node, node) == null) { counter.computeIfAbsent(node.getClass(), (__) -> new CopyOnWriteArrayList<>()).add(node); } + return new NodeWrapper(context, calls); } return null; } } + + private class NodeWrapper extends ExecutionEventNode { + + private final EventContext context; + + private final Map calls; + + public NodeWrapper(EventContext context, Map calls) { + this.context = context; + this.calls = calls; + } + + public void onReturnValue(VirtualFrame frame, Object result) { + Node node = context.getInstrumentedNode(); + if (node instanceof FunctionCallInstrumentationNode instrumentableNode + && result instanceof FunctionCallInstrumentationNode.FunctionCall functionCall) { + onFunctionReturn(instrumentableNode, functionCall); + } + } + + private void onFunctionReturn(FunctionCallInstrumentationNode node, FunctionCallInstrumentationNode.FunctionCall result) { + if (node.getId() != null) { + calls.put(node.getId(), new FunctionCallInfo(result)); + } + } + + } + + public class FunctionCallInfo { + + private final QualifiedName moduleName; + private final QualifiedName typeName; + private final String functionName; + + public FunctionCallInfo(FunctionCallInstrumentationNode.FunctionCall call) { + RootNode rootNode = call.getFunction().getCallTarget().getRootNode(); + if (rootNode instanceof MethodRootNode methodNode) { + moduleName = methodNode.getModuleScope().getModule().getName(); + typeName = methodNode.getType().getQualifiedName(); + functionName = methodNode.getMethodName(); + } else { + moduleName = null; + typeName = null; + functionName = rootNode.getName(); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + FunctionCallInfo that = (FunctionCallInfo) o; + return Objects.equals(moduleName, that.moduleName) + && Objects.equals(typeName, that.typeName) + && Objects.equals(functionName, that.functionName); + } + + @Override + public int hashCode() { + return Objects.hash(moduleName, typeName, functionName); + } + + @Override + public String toString() { + return moduleName + "::" + typeName + "::" + functionName; + } + + public QualifiedName getModuleName() { + return moduleName; + } + + public QualifiedName getTypeName() { + return typeName; + } + + public String getFunctionName() { + return functionName; + } + } } diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java index 31c267090f..e921a994fd 100644 --- a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java +++ b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java @@ -65,9 +65,7 @@ public class IncrementalUpdatesTest { sendUpdatesWhenFunctionBodyIsChangedBySettingValue("4", ConstantsGen.INTEGER, "4", "5", "5", LiteralNode.class); var m = context.languageContext().findModule(MODULE_NAME).orElse(null); assertNotNull("Module found", m); - var numbers = m.getIr().preorder().filter((v1) -> { - return v1 instanceof IR$Literal$Number; - }); + var numbers = m.getIr().preorder().filter((v1) -> v1 instanceof IR$Literal$Number); assertEquals("One number found: " + numbers, 1, numbers.size()); if (numbers.head() instanceof IR$Literal$Number n) { assertEquals("updated to 5", "5", n.value()); diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java new file mode 100644 index 0000000000..cf0003e35f --- /dev/null +++ b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java @@ -0,0 +1,92 @@ +package org.enso.interpreter.test.instrument; + +import com.oracle.truffle.api.instrumentation.SourceSectionFilter; +import com.oracle.truffle.api.instrumentation.StandardTags; +import org.enso.interpreter.runtime.tag.AvoidIdInstrumentationTag; +import org.enso.interpreter.runtime.tag.IdentifiedTag; +import org.enso.interpreter.test.Metadata; +import org.enso.interpreter.test.NodeCountingTestInstrument; +import org.enso.polyglot.RuntimeOptions; +import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.Language; +import org.graalvm.polyglot.Source; +import static org.junit.Assert.assertEquals; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.io.OutputStream; +import java.nio.file.Paths; +import java.util.Map; + +public class WarningInstrumentationTest { + + private Context context; + private NodeCountingTestInstrument instrument; + + @Before + public void initContext() { + context = Context.newBuilder() + .allowExperimentalOptions(true) + .option( + RuntimeOptions.LANGUAGE_HOME_OVERRIDE, + Paths.get("../../distribution/component").toFile().getAbsolutePath() + ) + .logHandler(OutputStream.nullOutputStream()) + .allowExperimentalOptions(true) + .allowIO(true) + .allowAllAccess(true) + .build(); + + var engine = context.getEngine(); + Map langs = engine.getLanguages(); + Assert.assertNotNull("Enso found: " + langs, langs.get("enso")); + + instrument = engine.getInstruments().get(NodeCountingTestInstrument.INSTRUMENT_ID).lookup(NodeCountingTestInstrument.class); + SourceSectionFilter builder = SourceSectionFilter.newBuilder() + .tagIs(StandardTags.ExpressionTag.class, StandardTags.CallTag.class) + .tagIs(IdentifiedTag.class) + .tagIsNot(AvoidIdInstrumentationTag.class) + .build(); + instrument.enable(builder); + } + + @After + public void disposeContext() { + context.close(); + } + + @Test + public void instrumentValueWithWarnings() throws Exception { + var metadata = new Metadata(); + + var idOp1 = metadata.addItem(151, 34, null); + var idOp2 = metadata.addItem(202, 31, null); + var idOp3 = metadata.addItem(250, 13, null); + var rawCode = """ + from Standard.Base import all + from Standard.Base.Warning import Warning + from Standard.Table.Data.Table import Table + + run column_name = + operator1 = Table.new [[column_name, [1,2,3]]] + operator2 = Warning.attach "Text" operator1 + operator3 = operator2.get + operator3 + """; + var code = metadata.appendToCode(rawCode); + var src = Source.newBuilder("enso", code, "TestWarning.enso").build(); + var module = context.eval(src); + var res = module.invokeMember("eval_expression", "run"); + res.execute("A"); + + var calls = instrument.registeredCalls(); + + assertEquals(calls.keySet().size(), 3); + assertEquals(calls.get(idOp1).getFunctionName(), "new"); + assertEquals(calls.get(idOp2).getFunctionName(), "attach"); + assertEquals(calls.get(idOp3).getTypeName().item(), "Table"); + assertEquals(calls.get(idOp3).getFunctionName(), "get"); + } +} diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/FunctionCallInstrumentationNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/FunctionCallInstrumentationNode.java index b7aaf3e5f5..4716cd1cec 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/FunctionCallInstrumentationNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/FunctionCallInstrumentationNode.java @@ -142,13 +142,15 @@ public class FunctionCallInstrumentationNode extends Node implements Instrumenta */ @Override public WrapperNode createWrapper(ProbeNode probeNode) { - return new FunctionCallInstrumentationNodeWrapper(this, probeNode); + var wrapper = new FunctionCallInstrumentationNodeWrapper(this, probeNode); + wrapper.setId(this.getId()); + return wrapper; } /** - * Makrs this node with relevant runtime tags. + * Marks this node with relevant runtime tags. * - * @param tag the tag to check agains. + * @param tag the tag to check against. * @return true if the node carries the {@code tag}, false otherwise. */ @Override diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeCallableNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeCallableNode.java index 1f0f88d1ef..3720b29fc1 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeCallableNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeCallableNode.java @@ -279,6 +279,7 @@ public abstract class InvokeCallableNode extends BaseNode { invokeFunctionNode.getDefaultsExecutionMode(), invokeFunctionNode.getArgumentsExecutionMode())); childDispatch.setTailStatus(getTailStatus()); + childDispatch.setId(invokeFunctionNode.getId()); notifyInserted(childDispatch); } } finally { @@ -356,5 +357,8 @@ public abstract class InvokeCallableNode extends BaseNode { invokeFunctionNode.setId(id); invokeMethodNode.setId(id); invokeConversionNode.setId(id); + if (childDispatch != null) { + childDispatch.setId(id); + } } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeConversionNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeConversionNode.java index 03757b1555..870dad08a7 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeConversionNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeConversionNode.java @@ -164,6 +164,7 @@ public abstract class InvokeConversionNode extends BaseNode { invokeFunctionNode.getArgumentsExecutionMode(), thatArgumentPosition)); childDispatch.setTailStatus(getTailStatus()); + childDispatch.setId(invokeFunctionNode.getId()); notifyInserted(childDispatch); } } finally { 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 index 1b4a81301b..79fddcd65b 100644 --- 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 @@ -156,6 +156,7 @@ public abstract class InvokeMethodNode extends BaseNode { invokeFunctionNode.getArgumentsExecutionMode(), thisArgumentPosition)); childDispatch.setTailStatus(getTailStatus()); + childDispatch.setId(invokeFunctionNode.getId()); notifyInserted(childDispatch); } } finally { @@ -525,5 +526,8 @@ public abstract class InvokeMethodNode extends BaseNode { */ public void setId(UUID id) { invokeFunctionNode.setId(id); + if (childDispatch != null) { + childDispatch.setId(id); + } } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/dispatch/InvokeFunctionNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/dispatch/InvokeFunctionNode.java index 1c7dacd86b..f9f5fd5bec 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/dispatch/InvokeFunctionNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/dispatch/InvokeFunctionNode.java @@ -189,4 +189,9 @@ public abstract class InvokeFunctionNode extends BaseNode { public void setId(UUID id) { functionCallInstrumentationNode.setId(id); } + + /** Returns expression ID of this node. */ + public UUID getId() { + return functionCallInstrumentationNode.getId(); + } }