PanicSentinel value isn't propagated via Vector & co.

[jdunkerley]

If the user enters some invalid code within a Vector, this results in only an "Execution failed" and no way to find the error.

[JaroslavTulach]

When I have t.enso file with:

from Standard.Base import all

main = [1, 2, 3, Decimal.nan ]

I can run it and see reasonable error:

sbt:enso> runEngineDistribution --run t.enso
Execution finished with an error: Method `nan` of type Decimal.type could not be found.
        at <enso> t.main(t.enso:3:18-28)

e.g. from a CLI perspective things are looking fine.

[JaroslavTulach]

The IDE converts the execution error cause by Decimal.nan into PanicSentinel according to its documentation:

This tracing is enabled by the active intervention of the runtime instrumentation, and does
not function in textual mode.

This tracing isn't going to happen in the CLI - only IdExecutionInstrument does the conversion.

However PanicSentinel isn't a warning - e.g. it doesn't get propagated further...

[JaroslavTulach]

What is the desired behavior? Shall we treat PanicSentinel as a Warning and propagate it like that? I can easily achieve

with following few lines change:

diff --git engine/runtime-instrument-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java engine/runtime-instrument-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
index c868592d51..a710420955 100644
--- engine/runtime-instrument-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
+++ engine/runtime-instrument-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
@@ -244,7 +248,10 @@ public class IdExecutionInstrument extends TruffleInstrument implements IdExecut
       if (exception instanceof TailCallException) {
         onTailCallReturn(exception, Function.ArgumentsHelper.getState(frame.getArguments()));
       } else if (exception instanceof PanicException panicException) {
-        onReturnValue(frame, new PanicSentinel(panicException, context.getInstrumentedNode()));
+        var ctx = EnsoContext.get(context.getInstrumentedNode());
+        var sentinel = new PanicSentinel(panicException, context.getInstrumentedNode());
+        var warning = Warning.create(ctx, panicException.getMessage(), context.getInstrumentedNode());
+        onReturnValue(frame, WithWarnings.appendTo(ctx, sentinel, warning));
       } else if (exception instanceof AbstractTruffleException) {
         onReturnValue(frame, exception);
       }

Would you consider that a fix, @jdunkerley? Probably not as I now can see that I changed the red border to orange one. If that is a problem (which I expect), then we need something slightly better:

• duplicate the treatment of WithWarnings everywhere to also deal with PanicSentinel
• also check the payload of WithWarnings in the language server and send message that will red border the nodes

CCing @4e6

[JaroslavTulach]

The #7385 pull request shows a conservative approach that just turns PanicSentinel into an object that has WarningsLibrary.hasWarnings(...). The more radical change is shown at jtulach/NoPanicSentinel branch - it removes PanicSentinel altogether - but then we'd have to fix a lot of Runtime*Tests. CCing @4e6. A sample difference for example is:

List(Response(Some(0823c808-30ab-44bd-9e80-3c0cf7f32166), PushContextResponse(55b911ed-f384-460a-9410-da16d121085b)), Response(None, ExecutionUpdate(55b911ed-f384-460a-9410-da16d121085b, List(Diagnostic(Error, Some("The name `undefined` could not be found."), Some(/tmp/enso-test-packages2956044925097422099/src/Main.enso), Some(Range(Position(2, 8), Position(2, 17))), Some(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a), Vector())))), Response(None, ExpressionUpdates(55b911ed-f384-460a-9410-da16d121085b, Set(ExpressionUpdate(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a, Some("Standard.Builtins.Main.Panic"), None, Vector(ExecutionTime(0)), false, true, Panic("Compile_Error.Error", List(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a)))))), Response(None, ExecutionFailed(55b911ed-f384-460a-9410-da16d121085b, Diagnostic(Error, Some("Compile_Error.Error"), Some(/tmp/enso-test-packages2956044925097422099/src/Main.enso), Some(Range(Position(2, 8), Position(2, 17))), Some(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a), Vector(StackTraceElement("Main.main", Some(/tmp/enso-test-packages2956044925097422099/src/Main.enso), Some(Range(Position(2, 8), Position(2, 17))), Some(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a)))))), Response(None, BackgroundJobsStartedNotification()), Response(None, BackgroundJobsStartedNotification())) 



did not contain the same elements as 


List(Response(None, BackgroundJobsStartedNotification()), Response(Some(0823c808-30ab-44bd-9e80-3c0cf7f32166), PushContextResponse(55b911ed-f384-460a-9410-da16d121085b)), Response(None, ExecutionUpdate(55b911ed-f384-460a-9410-da16d121085b, List(Diagnostic(Error, Some("The name `undefined` could not be found."), Some(/tmp/enso-test-packages2956044925097422099/src/Main.enso), Some(Range(Position(2, 8), Position(2, 17))), Some(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a), Vector())))), Response(None, ExpressionUpdates(55b911ed-f384-460a-9410-da16d121085b, Set(ExpressionUpdate(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a, Some("Standard.Builtins.Main.Panic"), None, Vector(ExecutionTime(0)), false, true, Panic("Compile_Error.Error", List(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a)))))), Response(None, ExpressionUpdates(55b911ed-f384-460a-9410-da16d121085b, Set(ExpressionUpdate(11ba8aab-a7fd-4160-bf93-35c836273df9, Some("Standard.Builtins.Main.Panic"), None, Vector(ExecutionTime(0)), false, true, Panic("Compile_Error.Error", List(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a)))))), Response(None, ExpressionUpdates(55b911ed-f384-460a-9410-da16d121085b, Set(ExpressionUpdate(115feac2-51fc-4191-8393-ea75a0f5e80b, Some("Standard.Builtins.Main.Panic"), None, Vector(ExecutionTime(0)), false, true, Panic("Compile_Error.Error", List(c75803f2-3fb3-426f-bb8a-c7bbec6cae3a)))))), Response(None, ExecutionComplete(55b911ed-f384-460a-9410-da16d121085b))) (RuntimeErrorsTest.scala:161)

e.g. we are not getting ExecutionFailed, but rather ExpressionUpdates because the commit removes many places where PanicSentinel gets thrown during a method invocation.

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-07-25):

Progress: Jul 25 - 9 h

• Propagate panic sentinel as warnings: Check for PanicSentinel in SequenceLiteralNode #7385 ready for review
• alternative fix: PanicSentinel value isn't propagated via Vector & co. #7245 (comment)
• cleaning the codebase: Moving PackageRepository.Default into its own EnsoPackageRepository #7395
• Investigate Python Interop #7387: Python date tests: Verify Python date can be used where Enso date #7396
• thinking about design of: Confusing handling of named arguments #7359
• note on holes: Ascribed types & atom_with_hole #7348 (comment)
• processing emails
• reviews: Invoke all Enso benchmarks via JMH #7101 (comment)
• discussing classpath: https://discord.com/channels/@me/1010161953634791434/1133418456272863252
• planning next iteration
• reproducing failure: https://discord.com/channels/@me/948519891961544714/1133344467752071178 It should be finished by 2023-07-28.

Next Day: Bugfixes & cleanup

Discord

Discord - A New Way to Chat with Friends & Communities

Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

Discord

Discord - A New Way to Chat with Friends & Communities

Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-07-27):

Progress: - SequenceLiteralNode is green: #7385

• review of annotation processor: Invoke all Enso benchmarks via JMH #7101 (comment)
• bench-lib/benchOnly support: 5f4d3bc
• reported a is None issue for GraalVM Python: InteropLibrary.isNull(foreignObject) isn't identical to foreignObject is None oracle/graalpython#345
• review of project rename: Allow users to give a project other than Upper_Snake_Case name #7397 (review)
• fighting with Python_Zone: Verify Python date can be used where Enso date #7396 (comment)
• refactoring module thought: Renaming Variable or Function Support within a Single File #7389 (comment)
• status of Investigate Python Interop #7387 (comment)
• emails & reviews & meetings It should be finished by 2023-07-28.

Next Day: Interop with Python

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-07-28):

Progress: - merged Python interop, phase I: #7396

• BigInteger PR: Adopting GraalVM's support for BigInteger #7420
• let Verify Python date can be used where Enso date #7396 pass the gate
• Debugging of benchmarks: 95e6dea
• TCRs&TCAs: Invoke all Enso benchmarks via JMH #7101 (review) It should be finished by 2023-07-28.

Next Day: BigInteger & Interop with Python