Skip to content

Commit

Permalink
Initialize AtomConstructor's fields via local vars
Browse files Browse the repository at this point in the history
The mechanism follows a similar approach to what is being in functions
with default arguments.
Additionally since InstantiateAtomNode wasn't a subtype of EnsoRootNode it
couldn't be used in the application, which was the primary reason for
issue #181449213.
Alternatively InstantiateAtomNode could have been enhanced to extend
EnsoRootNode rather than RootNode to carry scope info but the former
seemed simpler.

See test cases for previously crashing and invalid cases.
  • Loading branch information
hubertp committed Mar 16, 2022
1 parent 286950b commit 311cab0
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,5 @@ protected boolean isInstrumentable() {
public boolean isCloningAllowed() {
return true;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,21 @@
@NodeInfo(shortName = "Instantiate", description = "Instantiates a constant Atom constructor")
public class InstantiateNode extends ExpressionNode {
private final AtomConstructor constructor;
private @Children ExpressionNode[] arguments;
private @Children ExpressionNode[] argAssignments;
private @Children ExpressionNode[] argReads;
private @CompilationFinal(dimensions = 1) ConditionProfile[] profiles;
private @CompilationFinal(dimensions = 1) ConditionProfile[] warningProfiles;
private @CompilationFinal(dimensions = 1) BranchProfile[] sentinelProfiles;
private final ConditionProfile anyWarningsProfile = ConditionProfile.createCountingProfile();

InstantiateNode(AtomConstructor constructor, ExpressionNode[] arguments) {
InstantiateNode(AtomConstructor constructor, ExpressionNode[] argAssignments, ExpressionNode[] argReads) {
this.constructor = constructor;
this.arguments = arguments;
this.profiles = new ConditionProfile[arguments.length];
this.sentinelProfiles = new BranchProfile[arguments.length];
this.warningProfiles = new ConditionProfile[arguments.length];
for (int i = 0; i < arguments.length; ++i) {
this.argAssignments = argAssignments;
this.argReads = argReads;
this.profiles = new ConditionProfile[argAssignments.length];
this.sentinelProfiles = new BranchProfile[argAssignments.length];
this.warningProfiles = new ConditionProfile[argAssignments.length];
for (int i = 0; i < argAssignments.length; ++i) {
this.profiles[i] = ConditionProfile.createCountingProfile();
this.sentinelProfiles[i] = BranchProfile.create();
this.warningProfiles[i] = ConditionProfile.createCountingProfile();
Expand All @@ -44,11 +46,15 @@ public class InstantiateNode extends ExpressionNode {
* Creates an instance of this node.
*
* @param constructor the {@link AtomConstructor} this node will be instantiating
* @param arguments the expressions for field values
* @param argAssignments the expressions that evaluate and assign constructor arguments to local vars
* @param argReads the expressions that read field values from local vars
* @return a node that instantiates {@code constructor}
*/
public static InstantiateNode build(AtomConstructor constructor, ExpressionNode[] arguments) {
return new InstantiateNode(constructor, arguments);
public static InstantiateNode build(
AtomConstructor constructor,
ExpressionNode[] argAssignments,
ExpressionNode[] argReads) {
return new InstantiateNode(constructor, argAssignments, argReads);
}

/**
Expand All @@ -61,14 +67,15 @@ public static InstantiateNode build(AtomConstructor constructor, ExpressionNode[
@Override
@ExplodeLoop
public Object executeGeneric(VirtualFrame frame) {
Object[] argumentValues = new Object[arguments.length];
Object[] argumentValues = new Object[argAssignments.length];
boolean anyWarnings = false;
ArrayRope<Warning> accumulatedWarnings = new ArrayRope<>();
for (int i = 0; i < arguments.length; i++) {
for (int i = 0; i < argAssignments.length; i++) {
ConditionProfile profile = profiles[i];
ConditionProfile warningProfile = warningProfiles[i];
BranchProfile sentinelProfile = sentinelProfiles[i];
Object argument = arguments[i].executeGeneric(frame);
argAssignments[i].executeVoid(frame);
Object argument = argReads[i].executeGeneric(frame);
if (profile.profile(TypesGen.isDataflowError(argument))) {
return argument;
} else if (warningProfile.profile(argument instanceof WithWarnings)) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@
import com.oracle.truffle.api.library.ExportLibrary;
import com.oracle.truffle.api.library.ExportMessage;
import com.oracle.truffle.api.nodes.RootNode;
import org.enso.interpreter.Language;
import org.enso.interpreter.node.ClosureRootNode;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.node.callable.argument.ReadArgumentNode;
import org.enso.interpreter.node.expression.atom.GetFieldNode;
import org.enso.interpreter.node.expression.atom.InstantiateNode;
import org.enso.interpreter.node.expression.atom.QualifiedAccessorNode;
import org.enso.interpreter.node.expression.builtin.InstantiateAtomNode;
import org.enso.interpreter.node.expression.constant.ConstantObjectNode;
import org.enso.interpreter.runtime.Context;
import org.enso.interpreter.runtime.callable.UnresolvedConversion;
import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
import org.enso.interpreter.runtime.callable.argument.ArgumentDefinition;
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 @@ -39,7 +42,7 @@ public final class AtomConstructor implements TruffleObject {

/**
* Creates a new Atom constructor for a given name. The constructor is not valid until {@link
* AtomConstructor#initializeFields(ArgumentDefinition...)} is called.
* AtomConstructor#initializeFields(LocalScope,ExpressionNode[],ExpressionNode[],ArgumentDefinition...)} is called.
*
* @param name the name of the Atom constructor
* @param definitionScope the scope in which this constructor was defined
Expand All @@ -49,15 +52,39 @@ public AtomConstructor(String name, ModuleScope definitionScope) {
this.definitionScope = definitionScope;
}


/**
* Generates a constructor function for this {@link AtomConstructor}.
* Note that such manually constructed argument definitions must not have default arguments.
*
* @return {@code this}, for convenience
*/
public AtomConstructor initializeFields(ArgumentDefinition... args) {
ExpressionNode[] assignments = new ExpressionNode[args.length];
ExpressionNode[] reads = new ExpressionNode[args.length];
for (int i=0; i<args.length; i++) {
assignments[i] = ConstantObjectNode.build(null);
reads[i] = ReadArgumentNode.build(i, null);
}
return initializeFields(LocalScope.root(), assignments, reads, args);
}

/**
* Sets the fields of this {@link AtomConstructor} and generates a constructor function.
*
* @param localScope a description of the local scope
* @param assignments the expressions that evaluate and assign constructor arguments to local vars
* @param argReads the expressions that read field values from local vars
* @param args the arguments this constructor will take
* @return {@code this}, for convenience
*/
public AtomConstructor initializeFields(ArgumentDefinition... args) {
public AtomConstructor initializeFields(
LocalScope localScope,
ExpressionNode[] assignments,
ExpressionNode[] argReads,
ArgumentDefinition... args) {
CompilerDirectives.transferToInterpreterAndInvalidate();
this.constructorFunction = buildConstructorFunction(args);
this.constructorFunction = buildConstructorFunction(localScope, assignments, argReads, args);
generateMethods(args);
if (args.length == 0) {
cachedInstance = new Atom(this);
Expand All @@ -70,19 +97,22 @@ public AtomConstructor initializeFields(ArgumentDefinition... args) {
/**
* Generates a constructor function to be used for object instantiation from other Enso code.
*
* @param localScope a description of the local scope
* @param assignments the expressions that evaluate and assign constructor arguments to local vars
* @param argReads the expressions that read field values from local vars
* @param args the argument definitions for the constructor function to take
* @return a {@link Function} taking the specified arguments and returning an instance for this
* {@link AtomConstructor}
*/
private Function buildConstructorFunction(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);
private Function buildConstructorFunction(
LocalScope localScope,
ExpressionNode[] assignments,
ExpressionNode[] argReads,
ArgumentDefinition[] args) {
ExpressionNode instantiateNode = InstantiateNode.build(this, assignments, argReads);
RootNode rootNode =
InstantiateAtomNode.build(
null, definitionScope.getModule().getName().item() + "." + name, instantiateNode);
ClosureRootNode.build(null, localScope, definitionScope,instantiateNode,
instantiateNode.getSourceSection(), definitionScope.getModule().getName().item() + "." + name);
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,24 +159,41 @@ 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)
val argumentExpressions = new ArrayBuffer[(RuntimeExpression, RuntimeExpression)]

for (idx <- atomDefn.arguments.indices) {
argDefs(idx) = argFactory.run(atomDefn.arguments(idx), idx)
val unprocessedArg = atomDefn.arguments(idx)
val arg = argFactory.run(unprocessedArg, idx)
val occInfo = unprocessedArg
.unsafeGetMetadata(
AliasAnalysis,
"No occurrence on an argument definition."
)
.unsafeAs[AliasAnalysis.Info.Occurrence]
val slot = localScope.createVarSlot(occInfo.id)
argDefs(idx) = arg
val readArg = ReadArgumentNode.build(idx, arg.getDefaultValue.orElse(null))
val assignmentArg = AssignmentNode.build(readArg, slot)
val argRead = ReadLocalVariableNode.build(new FramePointer(0, slot))
argumentExpressions.append((assignmentArg, argRead))
}

atomCons.initializeFields(argDefs: _*)
val (assignments, reads) = argumentExpressions.unzip

atomCons.initializeFields(localScope, assignments.toArray, reads.toArray, 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 311cab0

Please sign in to comment.