Skip to content

Commit

Permalink
Unable to access constructor value in interactive mode (#8626)
Browse files Browse the repository at this point in the history
close #7184

The constructor value was not accessible because during the re-compilation a new instance of the type was registered in runtime. Then during the execution, an old cached instance of the type was used in method resolution.

Changelog:
- update: the registration of types in runtime
- update: invalidate cached nodes that became a resolution error after applying the edit
  • Loading branch information
4e6 authored Jan 3, 2024
1 parent 689c8f7 commit cfab344
Show file tree
Hide file tree
Showing 7 changed files with 600 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ public void modifyModuleSources(
rope -> {
logger.log(
Level.FINE,
"Applied edits. Source has {} lines, last line has {} characters",
"Applied edits. Source has {0} lines, last line has {1} characters.",
new Object[] {
rope.lines().length(),
rope.lines().drop(rope.lines().length() - 1).characters().length()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,18 @@ import com.oracle.truffle.api.TruffleLogger
import org.enso.compiler.CompilerResult
import org.enso.compiler.context._
import org.enso.compiler.core.Implicits.AsMetadata
import org.enso.compiler.core.IR
import org.enso.compiler.core.ir.{Diagnostic, IdentifiedLocation, Warning}
import org.enso.compiler.core.{ExternalID, IR}
import org.enso.compiler.core.ir.{
expression,
Diagnostic,
IdentifiedLocation,
Warning
}
import org.enso.compiler.core.ir.expression.Error
import org.enso.compiler.data.BindingsMap
import org.enso.compiler.pass.analyse.{
CachePreferenceAnalysis,
DataflowAnalysis,
GatherDiagnostics
}
import org.enso.interpreter.instrument.execution.{
Expand All @@ -30,7 +37,9 @@ import org.enso.polyglot.runtime.Runtime.Api.StackItem
import org.enso.text.buffer.Rope

import java.io.File
import java.util.UUID
import java.util.logging.Level

import scala.jdk.OptionConverters._

/** A job that ensures that specified files are compiled.
Expand Down Expand Up @@ -307,14 +316,15 @@ final class EnsureCompiledJob(
ctx.locking.releasePendingEditsLock()
logger.log(
Level.FINEST,
s"Kept pending edits lock [EnsureCompiledJob] for ${System.currentTimeMillis() - pendingEditsLockTimestamp} milliseconds"
"Kept pending edits lock [EnsureCompiledJob] for {} milliseconds",
System.currentTimeMillis() - pendingEditsLockTimestamp
)
ctx.locking.releaseFileLock(file)
logger.log(
Level.FINEST,
s"Kept file lock [EnsureCompiledJob] for ${System.currentTimeMillis() - fileLockTimestamp} milliseconds"
"Kept file lock [EnsureCompiledJob] for {} milliseconds",
System.currentTimeMillis() - fileLockTimestamp
)

}
}

Expand All @@ -329,9 +339,10 @@ final class EnsureCompiledJob(
changeset: Changeset[_],
ir: IR
): Seq[CacheInvalidation] = {
val resolutionErrors = findNodesWithResolutionErrors(ir)
val invalidateExpressionsCommand =
CacheInvalidation.Command.InvalidateKeys(
changeset.invalidated
changeset.invalidated ++ resolutionErrors
)
val moduleIds = ir.preorder().flatMap(_.location()).flatMap(_.id()).toSet
val invalidateStaleCommand =
Expand All @@ -350,6 +361,40 @@ final class EnsureCompiledJob(
)
}

/** Looks for the nodes with the resolution error and their dependents.
*
* @param ir the module IR
* @return the set of node ids affected by a resolution error in the module
*/
private def findNodesWithResolutionErrors(ir: IR): Set[UUID @ExternalID] = {
val metadata = ir
.unsafeGetMetadata(
DataflowAnalysis,
"Empty dataflow analysis metadata during the interactive compilation."
)

val resolutionNotFoundKeys =
ir.preorder()
.collect {
case err @ expression.errors.Resolution(
_,
expression.errors.Resolution
.ResolverError(BindingsMap.ResolutionNotFound),
_,
_
) =>
DataflowAnalysis.DependencyInfo.Type.Static(
err.getId,
err.getExternalId
)
}
.toSet

resolutionNotFoundKeys.flatMap(
metadata.dependents.getExternal(_).getOrElse(Set())
)
}

/** Run the invalidation commands.
*
* @param module the compiled module
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void setTailStatus(TailStatus isTail) {
* @return the results of evaluating the function arguments
*/
@ExplodeLoop
public Object[] evaluateArguments(VirtualFrame frame) {
private Object[] evaluateArguments(VirtualFrame frame) {
Object[] computedArguments = new Object[this.argExpressions.length];

for (int i = 0; i < this.argExpressions.length; ++i) {
Expand All @@ -93,8 +93,9 @@ public Object[] evaluateArguments(VirtualFrame frame) {
@Override
public Object executeGeneric(VirtualFrame frame) {
State state = Function.ArgumentsHelper.getState(frame.getArguments());
Object[] evaluatedArguments = evaluateArguments(frame);
return this.invokeCallableNode.execute(
this.callable.executeGeneric(frame), frame, state, evaluateArguments(frame));
this.callable.executeGeneric(frame), frame, state, evaluatedArguments);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ public boolean isEigenType() {

public void registerConstructor(AtomConstructor constructor) {
constructors.put(constructor.getName(), constructor);
gettersGenerated = false;
}

public Map<String, AtomConstructor> getConstructors() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ public ModuleScope(
this.exports = exports;
}

public void registerType(Type type) {
types.put(type.getName(), type);
public Type registerType(Type type) {
Type current = types.putIfAbsent(type.getName(), type);
return current == null ? type : current;
}

/**
Expand Down Expand Up @@ -109,7 +110,7 @@ private Map<String, Supplier<Function>> ensureMethodMapFor(Type type) {

private Map<String, Supplier<Function>> getMethodMapFor(Type type) {
Type tpeKey = type == null ? noTypeKey : type;
Map<String, Supplier<Function>> result = methods.get(type);
Map<String, Supplier<Function>> result = methods.get(tpeKey);
if (result == null) {
return new HashMap<>();
}
Expand Down Expand Up @@ -396,7 +397,6 @@ public void reset() {
imports = new HashSet<>();
exports = new HashSet<>();
methods = new HashMap<>();
types = new HashMap<>();
conversions = new HashMap<>();
polyglotSymbols = new HashMap<>();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,13 @@ class RuntimeStubsGenerator(builtins: Builtins) {
scope.registerType(builtinType.getType)
builtinType.getType.setShadowDefinitions(scope, true)
} else {
val rtp = if (tp.members.nonEmpty || tp.builtinType) {
Type.create(tp.name, scope, builtins.any(), builtins.any(), false)
} else {
Type.createSingleton(tp.name, scope, builtins.any(), false)
}
scope.registerType(rtp)
val createdType =
if (tp.members.nonEmpty || tp.builtinType) {
Type.create(tp.name, scope, builtins.any(), builtins.any(), false)
} else {
Type.createSingleton(tp.name, scope, builtins.any(), false)
}
val rtp = scope.registerType(createdType)
tp.members.foreach { cons =>
val constructor = new AtomConstructor(cons.name, scope, rtp)
rtp.registerConstructor(constructor)
Expand Down
Loading

0 comments on commit cfab344

Please sign in to comment.