Reading of uninitialized local variables is broken

[smarr]

Reading of uninitialized variables in methods is likely broken. At least it is in SOMns.

The following test fails with an assertion, because the read evaluates to null:

class Test usingPlatform: platform = Value ()(
  protected foo = (
    | a |
    a println.
    a := 1.
    a println.
  )

  public main: args = (
    foo.
    foo.
    ^ 0
  )
)

A fix could be:

diff --git a/src/som/compiler/MethodBuilder.java b/src/som/compiler/MethodBuilder.java
index 1598a2b..20e8757 100644
--- a/src/som/compiler/MethodBuilder.java
+++ b/src/som/compiler/MethodBuilder.java
@@ -101,7 +102,8 @@ public final class MethodBuilder {
     MethodScope outer = (outerBuilder != null)
         ? outerBuilder.getCurrentMethodScope()
         : null;
-    this.currentScope   = new MethodScope(new FrameDescriptor(), outer, clsScope);
+    assert Nil.nilObject != null;
+    this.currentScope   = new MethodScope(new FrameDescriptor(Nil.nilObject), outer, clsScope);

     accessesVariablesOfOuterScope = false;
     throwsNonLocalReturn          = false;

/cc @charig

[charig]

Stefan, good catch, same happens in TruffleMate and I guess in TruffleSOM. But, have you tried the proposed patch? Anyway, my impression is that each LocalVarNode* should have its own Slot (clone the slot at creation time) while they are sharing it right now.

[smarr]

The patch is for SOMns, it fixed the example above. I haven't tested it beyond that.

I am not sure exactly what you are referring to with the LocalVarNode* and having their own slot.
The slot represents the memory for a local variable. The LocalVarNode*s are there to access them. Cloning the slots does not make sense to me. What would that achieve? That would be similar to duplicating a field in an object for each place it is accessed, no?

The frame is actually tracking whether the slot is set. It has an array of tags. That could potentially also be used to mark a variable as unwritten again (for issue #5).

[charig]

Yes. What I said was complete nonsense. It would even be incorrect. What I was trying to think is something like this: protected foo = ( | a | a := nil. a println. a := 1. a println. )
The second LocalVarNode specializes to object because the FrameSlotKind is already set to object by nil. While debugging your example, I was thinking in a workaround to try to optimize this case but clearly duplicating FrameSlots is not the solution, sorry for the noise.

[charig]

If you want to update TruffleSOM it is quite straightforward but just in case I attach the commit and its regression test