Suspended default arguments cannot access previous arguments (including self)

[radeusgd]

This was a really nasty one to track down.

This repro demonstrates the problem:

from Standard.Base import all

type My_Type
    Value field
    
    member_method self regular_arg ~suspended_default_arg=("Text: {"+self.to_display_text+"}") =
        case regular_arg of
            "B" -> suspended_default_arg
            _ -> "OK"

    other_method self regular_arg not_suspended_default_arg=("Text: {"+self.to_display_text+"}") =
        case regular_arg of
            "B" -> not_suspended_default_arg
            _ -> "OK"

main =
    o = My_Type.Value 100

    IO.println ("With suspended default:")
    IO.println (o.member_method "A")
    IO.println (o.member_method "B")

    IO.println ("With regular default:")
    IO.println (o.other_method "A")
    IO.println (o.other_method "B")

Actual behaviour

With suspended default:
OK
Text: {Error: Uninitialized value}
With regular default:
OK
Text: {My_Type.Value}

As we can see, if the default argument is not suspended it all works OK - self is correct.

However, once the argument is suspended, the self within it now is uninitialized.

Expected behaviour

The behaviour should be the same regardless of if the argument is suspended or not.

[radeusgd]

When this bug gets fixed, we should revert the commit containing the workaround:
ffaa9f2

[radeusgd]

I've found out that this bug is more generic: it's not only impossible to access self, but any previous argument, as demonstrated by this example (analogous to the one above but instead of self we are reading an argument):

from Standard.Base import all

type My_Type
    Value field
    
    member_method self previous_arg regular_arg ~suspended_default_arg=("Text: {"+previous_arg.to_display_text+"}") =
        case regular_arg of
            "B" -> suspended_default_arg
            _ -> "OK: "+previous_arg.to_display_text

    other_method self previous_arg regular_arg not_suspended_default_arg=("Text: {"+previous_arg.to_display_text+"}") =
        case regular_arg of
            "B" -> not_suspended_default_arg
            _ -> "OK: "+previous_arg.to_display_text

main =
    o = My_Type.Value 100

    IO.println ("With suspended default:")
    IO.println (o.member_method "my previous arg" "A")
    IO.println (o.member_method "my previous arg" "B")

    IO.println ("With regular default:")
    IO.println (o.other_method "my previous arg" "A")
    IO.println (o.other_method "my previous arg" "B")

Actual behaviour

This script yields:

With suspended default:
OK: my previous arg
Text: {Error: Uninitialized value}
With regular default:
OK: my previous arg
Text: {my previous arg}

Expected behaviour

Obviously the ~ should not influence how bindings are resolved, and so the two examples should behave the same:

With suspended default:
OK: my previous arg
Text: {my previous arg}
With regular default:
OK: my previous arg
Text: {my previous arg}

[JaroslavTulach]

First investigation indicates, that this is caused by having wrong parentLevel() == 0 in getFramePointer() - if it was +1, then things might work better. CCing @Akirathan as a frame pointer expert. I was hoping this could be a fix:

$ git diff
diff --git engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
index 44c72557c8..0bc588e5cf 100644
--- engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
+++ engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
@@ -2302,8 +2302,9 @@ class IrToTruffle(
     ): ArgumentDefinition =
       inputArg match {
         case arg: DefinitionArgument.Specified =>
+          val suspendedScope = scope.createChild()
           val defaultExpression = arg.defaultValue
-            .map(new ExpressionProcessor(scope, scopeName).run(_, false))
+            .map(new ExpressionProcessor(suspendedScope, scopeName).run(_, false))
             .orNull
 
           // Note [Handling Suspended Defaults]
@@ -2311,7 +2312,7 @@ class IrToTruffle(
             assert(arg.defaultValue.isDefined)
             val defaultRootNode = ClosureRootNode.build(
               language,
-              scope,
+              suspendedScope,
               moduleScope,
               defaultExpression,
               makeSection(moduleScope, arg.defaultValue.get.location()),

but no luck. Probably I am doing something wrong when trying to use a new scope.createChild() for the thunk evaluation. Hints welcomed.

[JaroslavTulach]

I can fix the FramePointer to point to the right location in parent frame (to obtain value of self), but the problem is that there is no self when the lazy argument is being evaluated. I believe it may have something to do with #7571 (comment).

Here, at

Function$ArgumentsHelper.buildArguments() (org/enso/interpreter/runtime/callable/function/Function.java:328)
ThunkExecutorNode.doCached() (org/enso/interpreter/node/callable/thunk/ThunkExecutorNode.java:69)
ThunkExecutorNodeGen.executeAndSpecialize() (org/enso/interpreter/node/callable/thunk/ThunkExecutorNodeGen.java:208)
ThunkExecutorNodeGen.executeThunk() (org/enso/interpreter/node/callable/thunk/ThunkExecutorNodeGen.java:168)
ArgumentSorterNode.executeArguments() (org/enso/interpreter/node/callable/argument/ArgumentSorterNode.java:84)
ArgumentSorterNode.execute() (org/enso/interpreter/node/callable/argument/ArgumentSorterNode.java:100)

the thunk.getScope() contains a completely empty (materialized) frame. Probably the frame was materialized before (non-suspended) arguments were put into it. Such an initialization is happening at:

Function.thunk() (org/enso/interpreter/runtime/callable/function/Function.java:84)
CreateThunkNode.executeGeneric() (org/enso/interpreter/node/callable/thunk/CreateThunkNode.java:37)
ReadArgumentNode.executeGeneric() org/enso/interpreter/node/callable/argument/ReadArgumentNode.java:66)
AssignmentNodeGen.executeGeneric() (org/enso/interpreter/node/scope/AssignmentNodeGen.java:51)
StatementNode.executeGeneric() (org/enso/interpreter/node/callable/function/StatementNode.java:41)

which is obviously a bit too early as that is executed before AssignmentNode fills in the local variables.

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2024-05-28):

Progress: - suspended defaulted args PR created and merged: #10104

• FramePointer idea: https://discord.com/channels/@me/948519891961544714/1244856304477016114
• note about import/export: Add ExportImportResolutionBenchmark #10043 (comment)
• Additional persistance PR: Exposing just a single Persistance.Reference abstraction #10117 It should be finished by 2024-05-28.

Discord

Discord - A New Way to Chat with Friends & Communities

Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.