Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache dataflow errors #7193

Merged
merged 6 commits into from
Jul 9, 2023
Merged

Cache dataflow errors #7193

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
45c63b7
Cache dataflow errors
hubertp Jul 3, 2023
e3e65c2
Test for caching dataflow errors
hubertp Jul 3, 2023
d6a9038
changelog
hubertp Jul 3, 2023
1f4144d
fmt
hubertp Jul 3, 2023
a79f751
nits
hubertp Jul 5, 2023
89bf910
Revert "nits"
hubertp Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,7 @@
- [Add method call info for infix operators][7090]
- [`executionComplete` response is sent on successful execution only][7143]
- [Send info about function values][7168]
- [Cache dataflow errors][7193]

[3227]: https://github.com/enso-org/enso/pull/3227
[3248]: https://github.com/enso-org/enso/pull/3248
Expand Down Expand Up @@ -976,6 +977,7 @@
[7090]: https://github.com/enso-org/enso/pull/7090
[7143]: https://github.com/enso-org/enso/pull/7143
[7168]: https://github.com/enso-org/enso/pull/7168
[7193]: https://github.com/enso-org/enso/pull/7193

# Enso 2.0.0-alpha.18 (2021-10-12)

Expand Down
3 changes: 2 additions & 1 deletion ...ent-id-execution/src/main/java/org/enso/interpreter/instrument/IdExecutionInstrument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.control.TailCallException;
import org.enso.interpreter.runtime.data.Type;
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;
Expand Down Expand Up @@ -250,7 +251,7 @@ public void onReturnExceptional(VirtualFrame frame, Throwable exception) {
}

private void onExpressionReturn(Object result, Node node, EventContext context) throws ThreadDeath {
boolean isPanic = result instanceof AbstractTruffleException;
boolean isPanic = result instanceof AbstractTruffleException && !(result instanceof DataflowError);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any system in these checks at all (another example at https://github.com/enso-org/enso/pull/7205/files#diff-bfe95bc1ca4b016dccbab4fc97f23809a6c83d040de6ca8a6a615f02378e1a59R357)?

I am afraid we are just patching random parts of the Enso system without having a consistent classification of the objects and their roles. Everything seems to hold together just with some unit tests. At least we have them, but still...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a connection between this and the mentioned PR other than that we deal with DataFlowError. Yes, it's patching (not so random) parts of the Enso system and so what is wrong with that?
Alternatively we shouldn't match on AbstractTruffleException at all and wrap it into something we can control. Is it a better solution? That's debatable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is wrong with that?

It is perfectly pragmatic behavior. I am a huge fan of pragmatism. However sometimes I a masking myself - shouldn't we be more [rationalistic]http://wiki.apidesign.org/wiki/Rationalism) when designing a language?

Possibly more consistency than patching would make the language better...

UUID nodeId = ((ExpressionNode) node).getId();

String resultType = typeOf(result);
Expand Down
153 changes: 134 additions & 19 deletions ...h-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class RuntimeErrorsTest
| x = undefined
| y = foo x 42
| foo y 1
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -225,7 +225,7 @@ class RuntimeErrorsTest
| x = undefined
| y = foo x 42
| foo y 1
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -315,7 +315,7 @@ class RuntimeErrorsTest
"""foo a b = a + b + x
|
|main = foo 1 2
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -391,7 +391,7 @@ class RuntimeErrorsTest
|foo a b = a + b + x
|
|main = foo 1 2
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -476,7 +476,7 @@ class RuntimeErrorsTest
| x = Error.throw MyError
| y = foo x 42
| foo y 1
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -553,7 +553,7 @@ class RuntimeErrorsTest
| x = undefined
| y = 42
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -639,7 +639,7 @@ class RuntimeErrorsTest
| x = Error.throw MyError
| y = 42
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -723,7 +723,7 @@ class RuntimeErrorsTest
| x = Error.throw MyError
| y = x - 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -799,7 +799,20 @@ class RuntimeErrorsTest
3,
updatesOnlyFor = Set(xId, yId)
) should contain theSameElementsAs Seq(
TestMessages.update(contextId, xId, ConstantsGen.INTEGER),
TestMessages.update(
contextId,
xId,
ConstantsGen.INTEGER,
methodCall = Some(
Api.MethodCall(
Api.MethodPointer(
"Standard.Base.Error",
"Standard.Base.Error.Error",
"throw"
)
)
)
),
TestMessages.update(contextId, yId, ConstantsGen.INTEGER),
context.executionComplete(contextId)
)
Expand Down Expand Up @@ -885,7 +898,7 @@ class RuntimeErrorsTest
| x = Error.throw MyError1
| y = x - 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -988,7 +1001,7 @@ class RuntimeErrorsTest
| x = foo
| y = x - 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1080,7 +1093,7 @@ class RuntimeErrorsTest
| x = Panic.throw MyError
| y = x - 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1194,7 +1207,7 @@ class RuntimeErrorsTest
| x = 1 + foo
| y = x - 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1316,7 +1329,7 @@ class RuntimeErrorsTest
| x = Panic.throw MyError1
| y = x - 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1464,7 +1477,7 @@ class RuntimeErrorsTest
| x = foo
| y = x + 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1581,7 +1594,7 @@ class RuntimeErrorsTest
|main =
| x = foo
| x
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1652,7 +1665,7 @@ class RuntimeErrorsTest
| x = foo
| y = x + 1
| IO.println y
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1757,7 +1770,7 @@ class RuntimeErrorsTest
"""main =
| x = IO.println "MyError"
| x
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1869,7 +1882,7 @@ class RuntimeErrorsTest
|main =
| x = IO.println "MyError"
| x
|""".stripMargin.linesIterator.mkString("\n")
|""".stripMargin
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)

Expand Down Expand Up @@ -1964,4 +1977,106 @@ class RuntimeErrorsTest
context.consumeOut shouldEqual List("MyError")
}

it should "cache dataflow errors" in {
val contextId = UUID.randomUUID()
val requestId = UUID.randomUUID()
val moduleName = "Enso_Test.Test.Main"
val newline = "\n" // was: System.lineSeparator()

val metadata = new Metadata
val xId = metadata.addItem(111, 79)
val mainResId = metadata.addItem(195, 1)
val mainRes1Id = metadata.addItem(209, 1)

val code =
"""import Standard.Base.Error.Error
|import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
|
|main =
| x = Error.throw (Illegal_Argument.Error "The operation failed due to some reason.")
| x
|""".stripMargin
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 the new 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, "Enso_Test.Test.Main", "main"),
None,
Vector()
)
)
)
)
context.receiveNIgnorePendingExpressionUpdates(
5
) should contain theSameElementsAs Seq(
Api.Response(Api.BackgroundJobsStartedNotification()),
Api.Response(requestId, Api.PushContextResponse(contextId)),
TestMessages.error(
contextId,
xId,
methodCall = Api.MethodCall(
Api.MethodPointer(
"Standard.Base.Error",
"Standard.Base.Error.Error",
"throw"
)
),
Api.ExpressionUpdate.Payload.DataflowError(Seq(xId))
),
TestMessages.error(
contextId,
mainResId,
Api.ExpressionUpdate.Payload.DataflowError(Seq(xId))
),
context.executionComplete(contextId)
)
context.consumeOut shouldEqual Seq()

// Modify the file
context.send(
Api.Request(
Api.EditFileNotification(
mainFile,
Seq(
TextEdit(
model.Range(model.Position(5, 4), model.Position(5, 5)),
s"y = x - 1${newline} y"
)
),
execute = true
)
)
)
context.receiveNIgnorePendingExpressionUpdates(
2
) should contain theSameElementsAs Seq(
TestMessages.error(
contextId,
mainRes1Id,
Api.ExpressionUpdate.Payload.DataflowError(Seq(xId))
),
context.executionComplete(contextId)
)
context.consumeOut shouldEqual Seq()
}

}
7 changes: 7 additions & 0 deletions test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Errors/Illegal_Argument.enso
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import project.Nothing.Nothing
import project.Data.Text.Text

type Illegal_Argument
Error message cause=Nothing

to_display_text self = "Illegal Argument: "+self.message