Skip to content

Commit

Permalink
Only send suggestions updates when type changes (#6755)
Browse files Browse the repository at this point in the history
The change adds an additional field to `ExpressionUpdates` messages sent by `ProgramExecutionSupport` to indicate if the type of value (or its method pointer) has changed and therefore would potentially require a suggestions' update.

Prior to #3729 that check was done during the instrumentation. However we still want to continue to support "pending expression" functionality therefore `SuggestionsHandler` will use the additional information to filter only the required expression updates.

Most of the changes are related to adapting our tests to the new field.

Closes #6706.

# Important Notes
The associated project now loads and navigates smoothly.
Also attaching a screenshot from the project that illustrates that pending functionality continues to work:
[Kazam_screencast_00006.webm](https://github.com/enso-org/enso/assets/292128/35918841-f84f-4e1c-b1b0-40e45d97e111)
  • Loading branch information
hubertp authored May 18, 2023
1 parent 2bc7d34 commit a32a2ea
Show file tree
Hide file tree
Showing 12 changed files with 357 additions and 104 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,7 @@
- [Warning.get_all returns only unique warnings][6372]
- [Reimplement `enso_project` as a proper builtin][6352]
- [Limit number of reported warnings per value][6577]
- [Suggestions are updated only when the type of the expression changes][6755]

[3227]: https://github.com/enso-org/enso/pull/3227
[3248]: https://github.com/enso-org/enso/pull/3248
Expand Down Expand Up @@ -875,6 +876,7 @@
[6372]: https://github.com/enso-org/enso/pull/6372
[6352]: https://github.com/enso-org/enso/pull/6352
[6577]: https://github.com/enso-org/enso/pull/6577
[6755]: https://github.com/enso-org/enso/pull/6755

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ final class SuggestionsHandler(
updates.map(u => (u.expressionId, u.expressionType))
)
val types = updates.toSeq
.filter(_.typeChanged)
.flatMap(update => update.expressionType.map(update.expressionId -> _))
suggestionsRepo
.updateAll(types)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class ContextEventsListenerSpec
Some(methodPointer),
Vector(),
false,
true,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down Expand Up @@ -100,6 +101,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
true,
Api.ExpressionUpdate.Payload.DataflowError(
Seq(Suggestions.function.externalId.get)
)
Expand Down Expand Up @@ -139,6 +141,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
false,
Api.ExpressionUpdate.Payload.Panic("Method failure", Seq())
)
)
Expand Down Expand Up @@ -177,6 +180,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
false,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand All @@ -191,6 +195,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
false,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,8 @@ object Runtime {
* expression
* @param fromCache whether or not the value for this expression came
* from the cache
* @param typeChanged whether or not the type of the value or method definition
* has changed from the one that was cached, if any
* @param payload an extra information about the computed value
*/
case class ExpressionUpdate(
Expand All @@ -375,6 +377,7 @@ object Runtime {
methodCall: Option[MethodPointer],
profilingInfo: Vector[ProfilingInfo],
fromCache: Boolean,
typeChanged: Boolean,
payload: ExpressionUpdate.Payload
)
object ExpressionUpdate {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1363,23 +1363,29 @@ class RuntimeErrorsTest
Api.ExpressionUpdate.Payload.Panic(
"MyError2",
Seq(xId)
)
),
builtin = false,
typeChanged = false
),
TestMessages.panic(
contextId,
yId,
Api.ExpressionUpdate.Payload.Panic(
"MyError2",
Seq(xId)
)
),
builtin = false,
typeChanged = false
),
TestMessages.panic(
contextId,
mainResId,
Api.ExpressionUpdate.Payload.Panic(
"MyError2",
Seq(xId)
)
),
builtin = false,
typeChanged = false
),
context.executionComplete(contextId)
)
Expand Down Expand Up @@ -1492,10 +1498,14 @@ class RuntimeErrorsTest
contextId,
xId,
ConstantsGen.INTEGER,
Api.MethodPointer(moduleName, moduleName, "foo")
Api.MethodPointer(moduleName, moduleName, "foo"),
fromCache = false,
typeChanged = true
),
TestMessages.update(contextId, yId, ConstantsGen.INTEGER),
TestMessages.update(contextId, mainResId, ConstantsGen.NOTHING),
TestMessages
.update(contextId, yId, ConstantsGen.INTEGER, typeChanged = true),
TestMessages
.update(contextId, mainResId, ConstantsGen.NOTHING, typeChanged = true),
context.executionComplete(contextId)
)
context.consumeOut shouldEqual List("3")
Expand Down Expand Up @@ -1667,9 +1677,12 @@ class RuntimeErrorsTest
contextId,
xId,
ConstantsGen.INTEGER,
Api.MethodPointer(moduleName, moduleName, "foo")
Api.MethodPointer(moduleName, moduleName, "foo"),
fromCache = false,
typeChanged = true
),
TestMessages.update(contextId, yId, ConstantsGen.INTEGER),
TestMessages
.update(contextId, yId, ConstantsGen.INTEGER, typeChanged = true),
context.executionComplete(contextId)
)
context.consumeOut shouldEqual List("3")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ class RuntimeExecutionEnvironmentTest
Some(IF_ENABLED_METH_PTR),
Vector(Api.ProfilingInfo.ExecutionTime(0)),
false,
true,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down Expand Up @@ -320,6 +321,7 @@ class RuntimeExecutionEnvironmentTest
Some(IF_ENABLED_METH_PTR),
Vector(Api.ProfilingInfo.ExecutionTime(0)),
false,
true,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down
Loading

0 comments on commit a32a2ea

Please sign in to comment.