[Truffle] Proper implementation of Kernel#binding and Proc#binding.

* Creates a new frame with the surrounding frame as declaration frame. * Fix Binding#local_variable_{g,s}et to use the frame of the right level. * Shallow copy for Binding means copy only the first-level frame. (variables added in the copy are not reflected in original and vice-versa) * Make sure all FrameDescriptor default to nil.
jruby · Sep 16, 2015 · 23b4350 · 23b4350
1 parent bb747c7
commit 23b4350
Showing 8 changed files with 110 additions and 85 deletions.
diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/RubyNode.java b/truffle/src/main/java/org/jruby/truffle/nodes/RubyNode.java
@@ -14,6 +14,7 @@
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.dsl.ImportStatic;
 import com.oracle.truffle.api.dsl.TypeSystemReference;
+import com.oracle.truffle.api.frame.FrameDescriptor;
 import com.oracle.truffle.api.frame.MaterializedFrame;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.instrument.ProbeNode;
@@ -230,7 +231,8 @@ private MaterializedFrame setupFrame(VirtualFrame frame, Object... arguments) {
                         null,
                         RubyArguments.getSelf(frame.getArguments()),
                         null,
-                        new Object[] {}));
+                        new Object[] {}),
+                new FrameDescriptor(nil()));
 
         if (arguments.length % 2 == 1) {
             throw new UnsupportedOperationException("odd number of name-value pairs for arguments");

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/BindingNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/BindingNodes.java
@@ -11,7 +11,9 @@
 
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.ImportStatic;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.FrameDescriptor;
 import com.oracle.truffle.api.frame.FrameSlot;
@@ -36,6 +38,57 @@
 @CoreClass(name = "Binding")
 public abstract class BindingNodes {
 
+    public static DynamicObject createBinding(RubyContext context, MaterializedFrame frame) {
+        final Object[] arguments = frame.getArguments();
+
+        final MaterializedFrame bindingFrame = Truffle.getRuntime().createMaterializedFrame(
+                RubyArguments.pack(
+                        RubyArguments.getMethod(arguments),
+                        frame,
+                        RubyArguments.getSelf(arguments),
+                        RubyArguments.getBlock(arguments),
+                        RubyArguments.extractUserArguments(arguments)),
+                new FrameDescriptor(context.getCoreLibrary().getNilObject()));
+
+        return Layouts.BINDING.createBinding(context.getCoreLibrary().getBindingFactory(), bindingFrame);
+    }
+
+    protected static class FrameSlotAndDepth {
+        private final FrameSlot slot;
+        private final int depth;
+
+        public FrameSlotAndDepth(FrameSlot slot, int depth) {
+            this.slot = slot;
+            this.depth = depth;
+        }
+    }
+
+    public static FrameDescriptor getFrameDescriptor(DynamicObject binding) {
+        assert RubyGuards.isRubyBinding(binding);
+        return Layouts.BINDING.getFrame(binding).getFrameDescriptor();
+    }
+
+    public static FrameSlotAndDepth findFrameSlotOrNull(DynamicObject binding, DynamicObject symbol) {
+        assert RubyGuards.isRubyBinding(binding);
+        assert RubyGuards.isRubySymbol(symbol);
+
+        final String identifier = Layouts.SYMBOL.getString(symbol);
+
+        int depth = 0;
+        MaterializedFrame frame = Layouts.BINDING.getFrame(binding);
+
+        while (frame != null) {
+            final FrameSlot frameSlot = frame.getFrameDescriptor().findFrameSlot(identifier);
+            if (frameSlot != null) {
+                return new FrameSlotAndDepth(frameSlot, depth);
+            }
+
+            frame = RubyArguments.getDeclarationFrame(frame.getArguments());
+            depth++;
+        }
+        return null;
+    }
+
     @CoreMethod(names = { "dup", "clone" })
     public abstract static class DupNode extends UnaryCoreMethodNode {
 
@@ -50,12 +103,21 @@ public DupNode(RubyContext context, SourceSection sourceSection) {
         public DynamicObject dup(DynamicObject binding) {
             DynamicObject copy = allocateObjectNode.allocate(
                     Layouts.BASIC_OBJECT.getLogicalClass(binding),
-                    Layouts.BINDING.getFrame(binding));
+                    copyFrame(Layouts.BINDING.getFrame(binding)));
+            return copy;
+        }
+
+        private MaterializedFrame copyFrame(MaterializedFrame frame) {
+            final MaterializedFrame copy = Truffle.getRuntime().createMaterializedFrame(frame.getArguments(), frame.getFrameDescriptor().copy());
+            for (FrameSlot slot : frame.getFrameDescriptor().getSlots()) {
+                copy.setObject(copy.getFrameDescriptor().findFrameSlot(slot.getIdentifier()), frame.getValue(slot));
+            }
             return copy;
         }
 
     }
 
+    @ImportStatic(BindingNodes.class)
     @CoreMethod(names = "local_variable_get", required = 1)
     public abstract static class LocalVariableGetNode extends CoreMethodArrayArgumentsNode {
 
@@ -75,27 +137,27 @@ public LocalVariableGetNode(RubyContext context, SourceSection sourceSection) {
         public Object localVariableGetCached(DynamicObject binding, DynamicObject symbol,
                                              @Cached("symbol") DynamicObject cachedSymbol,
                                              @Cached("getFrameDescriptor(binding)") FrameDescriptor cachedFrameDescriptor,
-                                             @Cached("findFrameSlot(binding, symbol)") FrameSlot cachedFrameSlot,
+                                             @Cached("findFrameSlotOrNull(binding, symbol)") FrameSlotAndDepth cachedFrameSlot,
                                              @Cached("createReadNode(cachedFrameSlot)") ReadFrameSlotNode readLocalVariableNode) {
             if (cachedFrameSlot == null) {
                 CompilerDirectives.transferToInterpreter();
                 throw new RaiseException(getContext().getCoreLibrary().nameErrorLocalVariableNotDefined(Layouts.SYMBOL.getString(symbol), binding, this));
             } else {
-                return readLocalVariableNode.executeRead(Layouts.BINDING.getFrame(binding));
+                final MaterializedFrame frame = RubyArguments.getDeclarationFrame(Layouts.BINDING.getFrame(binding), cachedFrameSlot.depth);
+                return readLocalVariableNode.executeRead(frame);
             }
         }
 
         @TruffleBoundary
-        @Specialization(guards = {"isRubySymbol(symbol)", "!isLastLine(symbol)"})
+        @Specialization(guards = { "isRubySymbol(symbol)", "!isLastLine(symbol)" })
         public Object localVariableGetUncached(DynamicObject binding, DynamicObject symbol) {
-            final MaterializedFrame frame = Layouts.BINDING.getFrame(binding);
-            final FrameSlot frameSlot = frame.getFrameDescriptor().findFrameSlot(Layouts.SYMBOL.getString(symbol));
-
+            final FrameSlotAndDepth frameSlot = findFrameSlotOrNull(binding, symbol);
             if (frameSlot == null) {
                 throw new RaiseException(getContext().getCoreLibrary().nameErrorLocalVariableNotDefined(Layouts.SYMBOL.getString(symbol), binding, this));
+            } else {
+                final MaterializedFrame frame = RubyArguments.getDeclarationFrame(Layouts.BINDING.getFrame(binding), frameSlot.depth);
+                return frame.getValue(frameSlot.slot);
             }
-
-            return frame.getValue(frameSlot);
         }
 
         @TruffleBoundary
@@ -116,37 +178,11 @@ public Object localVariableGetLastLine(DynamicObject binding, DynamicObject symb
             }
         }
 
-        protected FrameDescriptor getFrameDescriptor(DynamicObject binding) {
-            assert RubyGuards.isRubyBinding(binding);
-            return Layouts.BINDING.getFrame(binding).getFrameDescriptor();
-        }
-
-        protected FrameSlot findFrameSlot(DynamicObject binding, DynamicObject symbol) {
-            assert RubyGuards.isRubyBinding(binding);
-            assert RubyGuards.isRubySymbol(symbol);
-
-            final String symbolString = Layouts.SYMBOL.getString(symbol);
-
-            MaterializedFrame frame = Layouts.BINDING.getFrame(binding);
-
-            while (frame != null) {
-                final FrameSlot frameSlot = frame.getFrameDescriptor().findFrameSlot(symbolString);
-
-                if (frameSlot != null) {
-                    return frameSlot;
-                }
-
-                frame = RubyArguments.getDeclarationFrame(frame.getArguments());
-            }
-
-            return null;
-        }
-
-        protected ReadFrameSlotNode createReadNode(FrameSlot frameSlot) {
+        protected ReadFrameSlotNode createReadNode(FrameSlotAndDepth frameSlot) {
             if (frameSlot == null) {
                 return null;
             } else {
-                return ReadFrameSlotNodeGen.create(frameSlot);
+                return ReadFrameSlotNodeGen.create(frameSlot.slot);
             }
         }
 
@@ -160,6 +196,7 @@ protected int getCacheLimit() {
 
     }
 
+    @ImportStatic(BindingNodes.class)
     @CoreMethod(names = "local_variable_set", required = 2)
     public abstract static class LocalVariableSetNode extends CoreMethodArrayArgumentsNode {
 
@@ -178,17 +215,19 @@ public LocalVariableSetNode(RubyContext context, SourceSection sourceSection) {
         }, limit = "getCacheLimit()")
         public Object localVariableSetCached(DynamicObject binding, DynamicObject symbol, Object value,
                                              @Cached("symbol") DynamicObject cachedSymbol,
-                                             @Cached("createWriteNode(findFrameSlot(binding, symbol))") WriteFrameSlotNode writeLocalVariableNode,
-                                             @Cached("getFrameDescriptor(binding)") FrameDescriptor cachedFrameDescriptor) {
-            return writeLocalVariableNode.executeWrite(Layouts.BINDING.getFrame(binding), value);
+                                             @Cached("getFrameDescriptor(binding)") FrameDescriptor cachedFrameDescriptor,
+                                             @Cached("findFrameSlot(binding, symbol)") FrameSlotAndDepth cachedFrameSlot,
+                                             @Cached("createWriteNode(cachedFrameSlot)") WriteFrameSlotNode writeLocalVariableNode) {
+            final MaterializedFrame frame = RubyArguments.getDeclarationFrame(Layouts.BINDING.getFrame(binding), cachedFrameSlot.depth);
+            return writeLocalVariableNode.executeWrite(frame, value);
         }
 
         @TruffleBoundary
-        @Specialization(guards = {"isRubySymbol(symbol)", "!isLastLine(symbol)"})
+        @Specialization(guards = { "isRubySymbol(symbol)", "!isLastLine(symbol)" })
         public Object localVariableSetUncached(DynamicObject binding, DynamicObject symbol, Object value) {
-            final MaterializedFrame frame = Layouts.BINDING.getFrame(binding);
-            final FrameSlot frameSlot = findFrameSlot(binding, symbol);
-            frame.setObject(frameSlot, value);
+            final FrameSlotAndDepth frameSlot = findFrameSlot(binding, symbol);
+            final MaterializedFrame frame = RubyArguments.getDeclarationFrame(Layouts.BINDING.getFrame(binding), frameSlot.depth);
+            frame.setObject(frameSlot.slot, value);
             return value;
         }
 
@@ -201,34 +240,18 @@ public Object localVariableSetLastLine(DynamicObject binding, DynamicObject symb
             return value;
         }
 
-        protected FrameDescriptor getFrameDescriptor(DynamicObject binding) {
-            assert RubyGuards.isRubyBinding(binding);
-            return Layouts.BINDING.getFrame(binding).getFrameDescriptor();
-        }
-
-        protected FrameSlot findFrameSlot(DynamicObject binding, DynamicObject symbol) {
-            assert RubyGuards.isRubyBinding(binding);
-            assert RubyGuards.isRubySymbol(symbol);
-
-            final String symbolString = Layouts.SYMBOL.getString(symbol);
-
-            MaterializedFrame frame = Layouts.BINDING.getFrame(binding);
-
-            while (frame != null) {
-                final FrameSlot frameSlot = frame.getFrameDescriptor().findFrameSlot(symbolString);
-
-                if (frameSlot != null) {
-                    return frameSlot;
-                }
-
-                frame = RubyArguments.getDeclarationFrame(frame.getArguments());
+        protected FrameSlotAndDepth findFrameSlot(DynamicObject binding, DynamicObject symbol) {
+            final FrameSlotAndDepth frameSlot = BindingNodes.findFrameSlotOrNull(binding, symbol);
+            if (frameSlot == null) {
+                final FrameSlot newSlot = Layouts.BINDING.getFrame(binding).getFrameDescriptor().addFrameSlot(Layouts.SYMBOL.getString(symbol));
+                return new FrameSlotAndDepth(newSlot, 0);
+            } else {
+                return frameSlot;
             }
-
-            return Layouts.BINDING.getFrame(binding).getFrameDescriptor().addFrameSlot(symbolString);
         }
 
-        protected WriteFrameSlotNode createWriteNode(FrameSlot frameSlot) {
-            return WriteFrameSlotNodeGen.create(frameSlot);
+        protected WriteFrameSlotNode createWriteNode(FrameSlotAndDepth frameSlot) {
+            return WriteFrameSlotNodeGen.create(frameSlot.slot);
         }
 
         protected boolean isLastLine(DynamicObject symbol) {

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java
@@ -290,11 +290,10 @@ public BindingNode(RubyContext context, SourceSection sourceSection) {
         @Specialization
         public DynamicObject binding() {
             // Materialize the caller's frame - false means don't use a slow path to get it - we want to optimize it
-
             final MaterializedFrame callerFrame = RubyCallStack.getCallerFrame(getContext())
                     .getFrame(FrameInstance.FrameAccess.MATERIALIZE, false).materialize();
 
-            return Layouts.BINDING.createBinding(getContext().getCoreLibrary().getBindingFactory(), callerFrame);
+            return BindingNodes.createBinding(getContext(), callerFrame);
         }
     }
 
@@ -591,7 +590,9 @@ public Object evalNilBinding(VirtualFrame frame, DynamicObject source, DynamicOb
         })
         public Object evalBinding(DynamicObject source, DynamicObject binding, NotProvided filename,
                                   NotProvided lineNumber) {
-            return getContext().eval(Layouts.STRING.getByteList(source), binding, false, this);
+            final Object result = getContext().eval(Layouts.STRING.getByteList(source), binding, false, this);
+            assert result != null;
+            return result;
         }
 
         @Specialization(guards = {

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/ProcNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/ProcNodes.java
@@ -210,10 +210,9 @@ public BindingNode(RubyContext context, SourceSection sourceSection) {
         }
 
         @Specialization
-        public Object binding(DynamicObject proc) {
+        public DynamicObject binding(DynamicObject proc) {
             final MaterializedFrame frame = Layouts.PROC.getDeclarationFrame(proc);
-
-            return Layouts.BINDING.createBinding(getContext().getCoreLibrary().getBindingFactory(), frame);
+            return BindingNodes.createBinding(getContext(), frame);
         }
 
     }

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/SymbolNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/SymbolNodes.java
@@ -144,7 +144,7 @@ protected DynamicObject createProc(VirtualFrame frame, DynamicObject symbol) {
 
             final RubyRootNode rootNode = new RubyRootNode(
                     getContext(), sourceSection,
-                    new FrameDescriptor(),
+                    new FrameDescriptor(nil()),
                     sharedMethodInfo,
                     SequenceNode.sequence(getContext(), sourceSection,
                             new CheckArityNode(getContext(), sourceSection, Arity.AT_LEAST_ONE),

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/array/ArrayNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/array/ArrayNodes.java
@@ -2306,7 +2306,7 @@ public static class MaxBlock {
         public MaxBlock(RubyContext context) {
             final SourceSection sourceSection = new CoreSourceSection("Array", "max");
 
-            frameDescriptor = new FrameDescriptor();
+            frameDescriptor = new FrameDescriptor(context.getCoreLibrary().getNilObject());
             frameSlot = frameDescriptor.addFrameSlot("maximum_memo");
 
             sharedMethodInfo = new SharedMethodInfo(sourceSection, null, Arity.NO_ARGUMENTS, "max", false, null, false);
@@ -2410,7 +2410,7 @@ public static class MinBlock {
         public MinBlock(RubyContext context) {
             final SourceSection sourceSection = new CoreSourceSection("Array", "min");
 
-            frameDescriptor = new FrameDescriptor();
+            frameDescriptor = new FrameDescriptor(context.getCoreLibrary().getNilObject());
             frameSlot = frameDescriptor.addFrameSlot("minimum_memo");
 
             sharedMethodInfo = new SharedMethodInfo(sourceSection, null, Arity.NO_ARGUMENTS, "min", false, null, false);

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/RubyArguments.java b/truffle/src/main/java/org/jruby/truffle/runtime/RubyArguments.java
@@ -153,7 +153,7 @@ public static MaterializedFrame getDeclarationFrame(VirtualFrame frame, int leve
      * current frame is 0.
      */
     @ExplodeLoop
-    private static MaterializedFrame getDeclarationFrame(MaterializedFrame frame, int level) {
+    public static MaterializedFrame getDeclarationFrame(MaterializedFrame frame, int level) {
         assert frame != null;
         assert level >= 0;
 

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java b/truffle/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java
@@ -171,8 +171,8 @@ public class CoreLibrary {
     private final DynamicObject rubiniusUndefined;
     private final DynamicObject digestClass;
 
-    private final ArrayNodes.MinBlock arrayMinBlock;
-    private final ArrayNodes.MaxBlock arrayMaxBlock;
+    @CompilationFinal private ArrayNodes.MinBlock arrayMinBlock;
+    @CompilationFinal private ArrayNodes.MaxBlock arrayMaxBlock;
 
     private final DynamicObject rubyInternalMethod;
     private final Map<Errno, DynamicObject> errnoClasses = new HashMap<>();
@@ -476,9 +476,6 @@ public CoreLibrary(RubyContext context) {
 
         globalVariablesObject = Layouts.CLASS.getInstanceFactory(objectClass).newInstance();
 
-        arrayMinBlock = new ArrayNodes.MinBlock(context);
-        arrayMaxBlock = new ArrayNodes.MaxBlock(context);
-
         digestClass = defineClass(truffleModule, basicObjectClass, "Digest");
         Layouts.CLASS.setInstanceFactoryUnsafe(digestClass, DigestLayoutImpl.INSTANCE.createDigestShape(digestClass, digestClass));
     }
@@ -509,6 +506,9 @@ public void initialize() {
     }
 
     private void addCoreMethods() {
+        arrayMinBlock = new ArrayNodes.MinBlock(context);
+        arrayMaxBlock = new ArrayNodes.MaxBlock(context);
+
         // Bring in core method nodes
         CoreMethodNodeManager coreMethodNodeManager = new CoreMethodNodeManager(objectClass, node.getSingletonClassNode());