Skip to content

Commit

Permalink
Carry partially evaluated arguments in frameslot
Browse files Browse the repository at this point in the history
In order to be able to refer to the previously defined parameters, the
frame will now carry such state in the additional `FrameSlot`.
In order to avoid paying the penalty of resolving dynamic symbols for
all closures, decided to bring back `InstantiateAtomNode`.

See test cases for the previously crashing examples.
  • Loading branch information
hubertp committed Mar 14, 2022
1 parent 6665b18 commit 4839e0a
Show file tree
Hide file tree
Showing 7 changed files with 150 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import org.enso.interpreter.runtime.scope.ModuleScope;
import org.enso.pkg.QualifiedName;

import java.util.Optional;

/** A common base class for all kinds of root node in Enso. */
@NodeInfo(shortName = "Root", description = "A root node for Enso computations")
public abstract class EnsoRootNode extends RootNode {
Expand Down Expand Up @@ -138,4 +140,13 @@ protected boolean isInstrumentable() {
public boolean isCloningAllowed() {
return true;
}

/**
* Gets the frame slot containing the already evaluated arguments.
*
* @return the arguments frame slot
*/
public Optional<FrameSlot> getArgsFrameSlot() {
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
package org.enso.interpreter.node.expression.atom;

import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
import com.oracle.truffle.api.frame.FrameSlot;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import com.oracle.truffle.api.nodes.NodeInfo;
import com.oracle.truffle.api.profiles.BranchProfile;
import com.oracle.truffle.api.profiles.ConditionProfile;
import org.enso.interpreter.node.EnsoRootNode;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.runtime.callable.argument.ArgumentDefinition;
import org.enso.interpreter.runtime.callable.atom.AtomConstructor;
import org.enso.interpreter.runtime.type.TypesGen;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

/**
* A node instantiating a constant {@link AtomConstructor} with values computed based on the
* children nodes.
Expand Down Expand Up @@ -54,10 +61,15 @@ public static InstantiateNode build(AtomConstructor constructor, ExpressionNode[
@ExplodeLoop
public Object executeGeneric(VirtualFrame frame) {
Object[] argumentValues = new Object[arguments.length];
FrameSlot argsFrameSlot = ((EnsoRootNode) getRootNode()).getArgsFrameSlot().orElseThrow(() -> new IllegalStateException("invalid RootNode when instantiating constructor"));
ArgumentDefinition[] fields = constructor.getFields();
Map<String, Object> evaluatedArgs = new HashMap<>();
for (int i = 0; i < arguments.length; i++) {
ConditionProfile profile = profiles[i];
BranchProfile sentinelProfile = sentinelProfiles[i];
Object argument = arguments[i].executeGeneric(frame);
evaluatedArgs.put(fields[i].getName(), argument);
frame.setObject(argsFrameSlot, evaluatedArgs);
if (profile.profile(TypesGen.isDataflowError(argument))) {
return argument;
} else if (TypesGen.isPanicSentinel(argument)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,37 @@
package org.enso.interpreter.node.expression.builtin;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.frame.FrameSlot;
import com.oracle.truffle.api.frame.FrameSlotKind;
import com.oracle.truffle.api.frame.FrameUtil;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.NodeInfo;
import com.oracle.truffle.api.nodes.RootNode;
import org.enso.interpreter.Language;
import org.enso.interpreter.node.EnsoRootNode;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.scope.LocalScope;
import org.enso.interpreter.runtime.scope.ModuleScope;
import org.enso.interpreter.runtime.state.Stateful;

import java.util.Optional;

/** This node represents the process of instantiating an atom at runtime. */
@NodeInfo(shortName = "constructor::", description = "An atom instantiation at runtime.")
public class InstantiateAtomNode extends RootNode {
public class InstantiateAtomNode extends EnsoRootNode {
private @Child ExpressionNode instantiator;
private final String name;
private final FrameSlot argsFrameSlot;

private InstantiateAtomNode(Language language, String name, ExpressionNode instantiator) {
super(language);
this.name = name;
private InstantiateAtomNode(
Language language,
LocalScope localScope,
ModuleScope moduleScope,
String name,
ExpressionNode instantiator) {
super(language, localScope, moduleScope, name, instantiator.getSourceSection());
this.instantiator = instantiator;
this.argsFrameSlot =
localScope.frameDescriptor().findOrAddFrameSlot("<<args_constructor>>", FrameSlotKind.Object);
}

/**
Expand All @@ -28,9 +42,14 @@ private InstantiateAtomNode(Language language, String name, ExpressionNode insta
*/
@Override
public Stateful execute(VirtualFrame frame) {
return new Stateful(
Function.ArgumentsHelper.getState(frame.getArguments()),
instantiator.executeGeneric(frame));
if (CompilerDirectives.inCompilationRoot() || CompilerDirectives.inInterpreter()) {
com.oracle.truffle.api.TruffleSafepoint.poll(this);
}
Object state = Function.ArgumentsHelper.getState(frame.getArguments());
frame.setObject(this.getStateFrameSlot(), state);
Object result = instantiator.executeGeneric(frame);
state = FrameUtil.getObjectSafe(frame, this.getStateFrameSlot());
return new Stateful(state, result);
}

/**
Expand All @@ -40,7 +59,7 @@ public Stateful execute(VirtualFrame frame) {
*/
@Override
public String getName() {
return "constructor::" + name;
return "constructor::" + getName();
}

/**
Expand All @@ -52,7 +71,20 @@ public String getName() {
* @return an instance of this node
*/
public static InstantiateAtomNode build(
Language language, String name, ExpressionNode instantiator) {
return new InstantiateAtomNode(language, name, instantiator);
Language language, LocalScope localScope,
ModuleScope moduleScope,
String name,
ExpressionNode instantiator) {
return new InstantiateAtomNode(language,localScope, moduleScope, name, instantiator);
}

/**
* Gets the frame slot containing the already evaluated arguments, if any..
*
* @return the arguments frame slot
*/
@Override
public Optional<FrameSlot> getArgsFrameSlot() {
return Optional.of(argsFrameSlot);
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
package org.enso.interpreter.node.expression.constant;

import com.oracle.truffle.api.frame.FrameSlot;
import com.oracle.truffle.api.frame.FrameUtil;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.NodeInfo;
import org.enso.interpreter.node.EnsoRootNode;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.runtime.callable.UnresolvedSymbol;

import java.util.Map;
import java.util.Optional;

/** Simple constant node that always results in the same {@link UnresolvedSymbol}. */
@NodeInfo(shortName = "DynamicSym")
public class DynamicSymbolNode extends ExpressionNode {
Expand Down Expand Up @@ -32,6 +38,16 @@ public static DynamicSymbolNode build(UnresolvedSymbol symbol) {
*/
@Override
public Object executeGeneric(VirtualFrame frame) {
return unresolvedSymbol;
Optional<FrameSlot> args = ((EnsoRootNode) getRootNode()).getArgsFrameSlot();
return args.flatMap(fs -> extractArgFromFrameSlot(FrameUtil.getObjectSafe(frame, fs), unresolvedSymbol.getName())).orElse(unresolvedSymbol);
}

private Optional<Object> extractArgFromFrameSlot(Object argMap, String symbolName) {
if (argMap != null ) {
Map<String, Object> mapping = (Map<String, Object>)argMap;
Object v = mapping.get(symbolName);
if (v != null) return Optional.of(v);
}
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.callable.function.FunctionSchema;
import org.enso.interpreter.runtime.library.dispatch.MethodDispatchLibrary;
import org.enso.interpreter.runtime.scope.LocalScope;
import org.enso.interpreter.runtime.scope.ModuleScope;
import org.enso.pkg.QualifiedName;

Expand All @@ -51,15 +52,27 @@ public AtomConstructor(String name, ModuleScope definitionScope) {
this.definitionScope = definitionScope;
}


/**
* Sets the fields of this {@link AtomConstructor} and generates a constructor function.
*
* @param args the arguments this constructor will take
* @return {@code this}, for convenience
*/
public AtomConstructor initializeFields(ArgumentDefinition... args) {
return initializeFieldsWithScope(LocalScope.root(), args);
}

/**
* Sets the fields of this {@link AtomConstructor} and generates a constructor function.
*
* @param localScope a description of the local scope
* @param args the arguments this constructor will take
* @return {@code this}, for convenience
*/
public AtomConstructor initializeFieldsWithScope(LocalScope localScope, ArgumentDefinition... args) {
CompilerDirectives.transferToInterpreterAndInvalidate();
this.constructorFunction = buildConstructorFunction(args);
this.constructorFunction = buildConstructorFunction(localScope, args);
generateMethods(args);
if (args.length == 0) {
cachedInstance = new Atom(this);
Expand All @@ -76,13 +89,13 @@ public AtomConstructor initializeFields(ArgumentDefinition... args) {
* @return a {@link Function} taking the specified arguments and returning an instance for this
* {@link AtomConstructor}
*/
private Function buildConstructorFunction(ArgumentDefinition[] args) {
private Function buildConstructorFunction(LocalScope localScope, ArgumentDefinition[] args) {
ExpressionNode[] argumentReaders = new ExpressionNode[args.length];
for (int i = 0; i < args.length; i++) {
argumentReaders[i] = ReadArgumentNode.build(i, args[i].getDefaultValue().orElse(null));
}
ExpressionNode instantiateNode = InstantiateNode.build(this, argumentReaders);
RootNode rootNode = InstantiateAtomNode.build(null, name, instantiateNode);
RootNode rootNode = InstantiateAtomNode.build(null, localScope, definitionScope, name, instantiateNode);
RootCallTarget callTarget = Truffle.getRuntime().createCallTarget(rootNode);
return new Function(callTarget, null, new FunctionSchema(args));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,16 @@ class IrToTruffle(
DataflowAnalysis,
"No dataflow information associated with an atom."
)
val localScope = new LocalScope(
None,
scopeInfo.graph,
scopeInfo.graph.rootScope,
dataflowInfo
)

val argFactory =
new DefinitionArgumentProcessor(
scope = new LocalScope(
None,
scopeInfo.graph,
scopeInfo.graph.rootScope,
dataflowInfo
)
scope = localScope
)
val argDefs =
new Array[ArgumentDefinition](atomDefn.arguments.size)
Expand All @@ -176,7 +177,7 @@ class IrToTruffle(
argDefs(idx) = argFactory.run(atomDefn.arguments(idx), idx)
}

atomCons.initializeFields(argDefs: _*)
atomCons.initializeFieldsWithScope(localScope, argDefs: _*)
}

// Register the method definitions in scope
Expand Down
42 changes: 42 additions & 0 deletions test/Tests/src/Semantic/Default_Args_Spec.enso
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from Standard.Base import all
import Standard.Test

type Box
type Foo (v : Bool = True)

type Bar (a : Integer = 1) (b : Box = (Foo False))

type A a=0 b=1
type B a=2 b=(Foo True)
type C a=3 b=Foo
type D a=4 b=(Bar 1)
type E a=5 b=a c=(b+1)
type F a=6 b=(Foo False) c=(b.v)
#type D a=4 b=Bar // will crash

spec =
Test.group "Atom Constructors" <|
Test.specify "should be allowed to use primitive default arguments" <|
x = A 1
x.b.should_equal 1
y = A 1
y.b.should_equal 1

Test.specify "should be allowed to use non-primitive default arguments" <|
a = B 1 (Foo False)
a.b.should_equal (Foo False)
b = B 1
b.b.should_equal (Foo True)
c = C 1
c.b.should_equal (Foo)
d = D 1
d.b.b.should_equal (Foo False)

Test.specify "should be allowed to use default arguments that refer to previous parameters" <|
e = E 1
e.b.should_equal 1
e.c.should_equal 2
f = F 1
f.c.should_equal False

main = Test.Suite.run_main here.spec

0 comments on commit 4839e0a

Please sign in to comment.