Initialize AtomConstructor's fields via local vars (#3330)

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.

CHANGELOG.md

@@ -122,10 +122,12 @@
 - [Added overloaded `from` conversions.][3227]
 - [Upgraded to Graal VM 21.3.0][3258]
 - [Added the ability to decorate values with warnings.][3248]
+- [Fixed issues related to constructors' default arguments][3330]
 
 [3227]: https://github.com/enso-org/enso/pull/3227
 [3248]: https://github.com/enso-org/enso/pull/3248
 [3258]: https://github.com/enso-org/enso/pull/3258
+[3330]: https://github.com/enso-org/enso/pull/3330
 
 # Enso 2.0.0-alpha.18 (2021-10-12)
 

engine/runtime/src/main/java/org/enso/interpreter/node/EnsoRootNode.java

@@ -128,4 +128,5 @@ public abstract class EnsoRootNode extends RootNode {
   public boolean isCloningAllowed() {
     return true;
   }
+
 }

engine/runtime/src/main/java/org/enso/interpreter/node/expression/atom/InstantiateNode.java

@@ -44,7 +44,7 @@ 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 arguments the expressions that produce field values
    * @return a node that instantiates {@code constructor}
    */
   public static InstantiateNode build(AtomConstructor constructor, ExpressionNode[] arguments) {

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/InstantiateAtomNode.java

@@ -1,58 +0,0 @@
-package org.enso.interpreter.node.expression.builtin;
-
-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.ExpressionNode;
-import org.enso.interpreter.runtime.callable.function.Function;
-import org.enso.interpreter.runtime.state.Stateful;
-
-/** 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 {
-  private @Child ExpressionNode instantiator;
-  private final String name;
-
-  private InstantiateAtomNode(Language language, String name, ExpressionNode instantiator) {
-    super(language);
-    this.name = name;
-    this.instantiator = instantiator;
-  }
-
-  /**
-   * Executes this node.
-   *
-   * @param frame the language frame being executed
-   * @return the result of executing this node
-   */
-  @Override
-  public Stateful execute(VirtualFrame frame) {
-    return new Stateful(
-        Function.ArgumentsHelper.getState(frame.getArguments()),
-        instantiator.executeGeneric(frame));
-  }
-
-  /**
-   * Returns a string representation of this node.
-   *
-   * @return a string representation of this node
-   */
-  @Override
-  public String getName() {
-    return name;
-  }
-
-  /**
-   * Creates an instance of this node.
-   *
-   * @param language the language for which the node is created
-   * @param name the name of the atom being instantated
-   * @param instantiator the expression used to instantiate the atom
-   * @return an instance of this node
-   */
-  public static InstantiateAtomNode build(
-      Language language, String name, ExpressionNode instantiator) {
-    return new InstantiateAtomNode(language, name, instantiator);
-  }
-}

engine/runtime/src/main/java/org/enso/interpreter/runtime/callable/atom/AtomConstructor.java

@@ -11,12 +11,13 @@ import com.oracle.truffle.api.interop.TruffleObject;
 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.node.ClosureRootNode;
 import org.enso.interpreter.node.ExpressionNode;
 import org.enso.interpreter.node.callable.argument.ReadArgumentNode;
+import org.enso.interpreter.node.callable.function.BlockNode;
 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.runtime.Context;
 import org.enso.interpreter.runtime.callable.UnresolvedConversion;
 import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
@@ -24,6 +25,7 @@ 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;
 
@@ -39,7 +41,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
@@ -49,15 +51,37 @@ public final class AtomConstructor implements TruffleObject {
     this.definitionScope = definitionScope;
   }
 
+
   /**
-   * Sets the fields of this {@link AtomConstructor} and generates a constructor function.
+   * Generates a constructor function for this {@link AtomConstructor}.
+   * Note that such manually constructed argument definitions must not have default arguments.
    *
-   * @param args the arguments this constructor will take
    * @return {@code this}, for convenience
    */
   public AtomConstructor initializeFields(ArgumentDefinition... args) {
+    ExpressionNode[] reads = new ExpressionNode[args.length];
+    for (int i=0; i<args.length; i++) {
+      reads[i] = ReadArgumentNode.build(i, null);
+    }
+    return initializeFields(LocalScope.root(), new ExpressionNode[0], 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 varReads 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(
+          LocalScope localScope,
+          ExpressionNode[] assignments,
+          ExpressionNode[] varReads,
+          ArgumentDefinition... args) {
     CompilerDirectives.transferToInterpreterAndInvalidate();
-    this.constructorFunction = buildConstructorFunction(args);
+    this.constructorFunction = buildConstructorFunction(localScope, assignments, varReads, args);
     generateMethods(args);
     if (args.length == 0) {
       cachedInstance = new Atom(this);
@@ -69,20 +93,27 @@ public final class AtomConstructor implements TruffleObject {
 
   /**
    * Generates a constructor function to be used for object instantiation from other Enso code.
+   * Building constructor function involves storing the argument in a local var and then reading
+   * it again on purpose. That way default arguments can refer to previously defined constructor arguments.
    *
+   * @param localScope a description of the local scope
+   * @param assignments the expressions that evaluate and assign constructor arguments to local vars
+   * @param varReads the expressions that read field values from previously evaluated 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[] varReads,
+          ArgumentDefinition[] args) {
+
+    ExpressionNode instantiateNode = InstantiateNode.build(this, varReads);
+    BlockNode instantiateBlock = BlockNode.build(assignments, instantiateNode);
     RootNode rootNode =
-        InstantiateAtomNode.build(
-            null, definitionScope.getModule().getName().item() + "." + name, instantiateNode);
+        ClosureRootNode.build(null, localScope, definitionScope, instantiateBlock,
+                instantiateNode.getSourceSection(), definitionScope.getModule().getName().item() + "." + name);
     RootCallTarget callTarget = Truffle.getRuntime().createCallTarget(rootNode);
     return new Function(callTarget, null, new FunctionSchema(args));
   }

engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala

@@ -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

test/Tests/src/Semantic/Default_Args_Spec.enso

@@ -0,0 +1,43 @@
+from Standard.Base import all
+import Standard.Test
+
+type Box
+    type Foo (v : Bool = True)
+
+type Bar (a : Integer = 1) (b : Box = (Foo False)) (c : Boolean = b.v)
+
+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)
+            d.b.c.should_equal 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