jruby · Oct 2, 2015 · Oct 2, 2015 · Oct 2, 2015 · Oct 2, 2015 · Oct 2, 2015
diff --git a/spec/ruby/core/module/shared/set_visibility.rb b/spec/ruby/core/module/shared/set_visibility.rb
@@ -119,5 +119,19 @@ def test1() end
 
       mod.should send(:"have_#{@method}_instance_method", :test1, false)
     end
+
+    describe "within a closure" do
+      it "sets the visibility outside the closure" do
+        visibility = @method
+        mod = Module.new {
+              1.times {
+                send visibility
+              }
+              def test1() end
+            }
+
+        mod.should send(:"have_#{@method}_instance_method", :test1, false)
+      end
+    end
   end
 end
diff --git a/spec/ruby/language/fixtures/private.rb b/spec/ruby/language/fixtures/private.rb
@@ -16,6 +16,9 @@ def foo
     end
 
     private
+
+    def self.public_defs_method; 0; end
+
     class C
       def baz
         "baz"
@@ -27,7 +30,6 @@ def public_class_method1; 1; end
       private
       def private_class_method1; 1; end
     end
-    def self.public_class_method2; 2; end
 
     def bar
       "bar"

diff --git a/spec/ruby/language/private_spec.rb b/spec/ruby/language/private_spec.rb
@@ -12,12 +12,16 @@
     lambda { b.bar }.should raise_error(NoMethodError)
   end
 
+  # def expr.meth() methods are always public
+  it "has no effect on def expr.meth() methods" do
+    Private::B.public_defs_method.should == 0
+  end
+
   it "is overridden when a new class is opened" do
     c = Private::B::C.new
     c.methods.should include(:baz)
     c.baz
     Private::B.public_class_method1.should == 1
-    Private::B.public_class_method2.should == 2
     lambda { Private::B.private_class_method1 }.should raise_error(NoMethodError)
   end
 

diff --git a/spec/tags/ruby/core/module/private_tags.txt b/spec/tags/ruby/core/module/private_tags.txt
@@ -1 +1,2 @@
 fails:Module#private without arguments does not affect method definitions when itself is inside an eval and method definitions are outside
+fails:Module#private without arguments within a closure sets the visibility outside the closure
diff --git a/spec/tags/ruby/core/module/protected_tags.txt b/spec/tags/ruby/core/module/protected_tags.txt
@@ -1 +1,2 @@
 fails:Module#protected without arguments does not affect method definitions when itself is inside an eval and method definitions are outside
+fails:Module#protected without arguments within a closure sets the visibility outside the closure
diff --git a/spec/truffle/tags/core/kernel/public_method_tags.txt b/spec/truffle/tags/core/kernel/public_method_tags.txt
@@ -2,4 +2,3 @@ fails:Kernel#public_method returns a method object for a valid method
 fails:Kernel#public_method returns a method object for a valid singleton method
 fails:Kernel#public_method returns a method object if we repond_to_missing? method
 fails:Kernel#public_method changes the method called for super on a target aliased method
-fails:Kernel#public_method raises a NameError if we only repond_to_missing? method, true
diff --git a/spec/truffle/tags/core/module/private_tags.txt b/spec/truffle/tags/core/module/private_tags.txt
diff --git a/spec/truffle/tags/core/module/protected_tags.txt b/spec/truffle/tags/core/module/protected_tags.txt
diff --git a/spec/truffle/tags/core/module/public_tags.txt b/spec/truffle/tags/core/module/public_tags.txt
@@ -1,3 +1 @@
 fails:Module#public on a superclass method calls the redefined method
-fails:Module#public without arguments affects evaled method definitions when itself is outside the eval
-fails:Module#public without arguments affects normally if itself and following method definitions are inside a eval
diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/RubyRootNode.java b/truffle/src/main/java/org/jruby/truffle/nodes/RubyRootNode.java
@@ -45,6 +45,10 @@ public RubyRootNode(RubyContext context, SourceSection sourceSection, FrameDescr
         this.needsDeclarationFrame = needsDeclarationFrame;
     }
 
+    public RubyRootNode withBody(RubyNode body) {
+        return new RubyRootNode(context, getSourceSection(), getFrameDescriptor(), sharedMethodInfo, body, needsDeclarationFrame);
+    }
+
     @Override
     public boolean isCloningAllowed() {
         return true;

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
@@ -76,7 +76,6 @@
 import org.jruby.truffle.runtime.loader.FeatureLoader;
 import org.jruby.truffle.runtime.methods.InternalMethod;
 import org.jruby.truffle.runtime.subsystems.ThreadManager.BlockingAction;
-import org.jruby.truffle.translator.NodeWrapper;
 import org.jruby.truffle.translator.TranslatorDriver;
 import org.jruby.truffle.translator.TranslatorDriver.ParserContext;
 import org.jruby.util.ByteList;
@@ -676,12 +675,7 @@ protected RootNodeWrapper compileSource(VirtualFrame frame, DynamicObject source
 
             final TranslatorDriver translator = new TranslatorDriver(getContext());
 
-            return new RootNodeWrapper(translator.parse(getContext(), source, UTF8Encoding.INSTANCE, ParserContext.EVAL, parentFrame, true, this, new NodeWrapper() {
-                @Override
-                public RubyNode wrap(RubyNode node) {
-                    return node; // return new SetMethodDeclarationContext(node.getContext(), node.getSourceSection(), Visibility.PRIVATE, "simple eval", node);
-                }
-            }));
+            return new RootNodeWrapper(translator.parse(getContext(), source, UTF8Encoding.INSTANCE, ParserContext.EVAL, parentFrame, true, this));
         }
 
         protected boolean parseDependsOnDeclarationFrame(RootNodeWrapper rootNode) {

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/ModuleNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/ModuleNodes.java
@@ -47,7 +47,6 @@
 import org.jruby.truffle.nodes.methods.CanBindMethodToModuleNode;
 import org.jruby.truffle.nodes.methods.CanBindMethodToModuleNodeGen;
 import org.jruby.truffle.nodes.methods.DeclarationContext;
-import org.jruby.truffle.nodes.methods.SetMethodDeclarationContext;
 import org.jruby.truffle.nodes.objects.*;
 import org.jruby.truffle.nodes.yield.YieldDispatchHeadNode;
 import org.jruby.truffle.runtime.*;
@@ -60,8 +59,6 @@
 import org.jruby.truffle.runtime.methods.Arity;
 import org.jruby.truffle.runtime.methods.InternalMethod;
 import org.jruby.truffle.runtime.methods.SharedMethodInfo;
-import org.jruby.truffle.translator.NodeWrapper;
-import org.jruby.truffle.translator.TranslatorDriver;
 import org.jruby.truffle.translator.TranslatorDriver.ParserContext;
 import org.jruby.util.IdUtil;
 import org.jruby.util.StringSupport;
@@ -74,12 +71,6 @@
 @CoreClass(name = "Module")
 public abstract class ModuleNodes {
 
-    /**
-     * The slot within a module definition method frame where we store the implicit state that is
-     * the current visibility for new methods.
-     */
-    public static final Object VISIBILITY_FRAME_SLOT_ID = new Object();
-
     public static DynamicObject createRubyModule(RubyContext context, DynamicObject selfClass, DynamicObject lexicalParent, String name, Node currentNode) {
         final ModuleFields model = new ModuleFields(context, lexicalParent, name);
         final DynamicObject module = Layouts.MODULE.createModule(Layouts.CLASS.getInstanceFactory(selfClass), model);
@@ -401,7 +392,7 @@ public DynamicObject generateAccessor(VirtualFrame frame, DynamicObject module,
             CompilerDirectives.transferToInterpreter();
             final FrameInstance callerFrame = RubyCallStack.getCallerFrame(getContext());
             final SourceSection sourceSection = callerFrame.getCallNode().getEncapsulatingSourceSection();
-            final Visibility visibility = AddMethodNode.getVisibility(callerFrame.getFrame(FrameAccess.READ_ONLY, true));
+            final Visibility visibility = DeclarationContext.findVisibility(callerFrame.getFrame(FrameAccess.READ_ONLY, true));
             final Arity arity = isGetter ? Arity.NO_ARGUMENTS : Arity.ONE_REQUIRED;
             final String ivar = "@" + name;
             final String accessorName = isGetter ? name : name + "=";
@@ -654,12 +645,7 @@ private Object classEvalSource(DynamicObject module, DynamicObject code, String
             CompilerDirectives.transferToInterpreter();
             Source source = Source.fromText(code.toString(), file);
 
-            return getContext().execute(source, encoding, ParserContext.MODULE, module, callerFrame, true, DeclarationContext.CLASS_EVAL, this, new NodeWrapper() {
-                @Override
-                public RubyNode wrap(RubyNode node) {
-                    return new SetMethodDeclarationContext(node.getContext(), node.getSourceSection(), Visibility.PUBLIC, "class_eval", node);
-                }
-            });
+            return getContext().parseAndExecute(source, encoding, ParserContext.MODULE, module, callerFrame, true, DeclarationContext.CLASS_EVAL, this);
         }
 
         @Specialization(guards = "isRubyProc(block)")
@@ -1936,16 +1922,8 @@ public DynamicObject setVisibility(VirtualFrame frame, DynamicObject module, Obj
         }
 
         private void setCurrentVisibility(Visibility visibility) {
-            CompilerDirectives.transferToInterpreter();
-
             final Frame callerFrame = RubyCallStack.getCallerFrame(getContext()).getFrame(FrameInstance.FrameAccess.READ_WRITE, true);
-            assert callerFrame != null;
-            assert callerFrame.getFrameDescriptor() != null;
-
-            final FrameSlot visibilitySlot = callerFrame.getFrameDescriptor().findOrAddFrameSlot(
-                    VISIBILITY_FRAME_SLOT_ID, "visibility for frame", FrameSlotKind.Object);
-
-            callerFrame.setObject(visibilitySlot, visibility);
+            DeclarationContext.changeVisibility(callerFrame, visibility);
         }
 
     }

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/methods/AddMethodNode.java b/truffle/src/main/java/org/jruby/truffle/nodes/methods/AddMethodNode.java
@@ -11,31 +11,31 @@
 
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.frame.Frame;
-import com.oracle.truffle.api.frame.FrameSlot;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.object.DynamicObject;
 import com.oracle.truffle.api.source.SourceSection;
 
 import org.jruby.runtime.Visibility;
 import org.jruby.truffle.nodes.RubyGuards;
 import org.jruby.truffle.nodes.RubyNode;
-import org.jruby.truffle.nodes.core.ModuleNodes;
 import org.jruby.truffle.nodes.objects.SingletonClassNode;
 import org.jruby.truffle.nodes.objects.SingletonClassNodeGen;
 import org.jruby.truffle.runtime.ModuleOperations;
-import org.jruby.truffle.runtime.RubyArguments;
 import org.jruby.truffle.runtime.RubyContext;
 import org.jruby.truffle.runtime.layouts.Layouts;
 import org.jruby.truffle.runtime.methods.InternalMethod;
 
 public class AddMethodNode extends RubyNode {
 
+    private final boolean isDefs; // def expr.meth()
+
     @Child private RubyNode receiver;
     @Child private MethodDefinitionNode methodNode;
     @Child private SingletonClassNode singletonClassNode;
 
-    public AddMethodNode(RubyContext context, SourceSection section, RubyNode receiver, MethodDefinitionNode method) {
+    public AddMethodNode(RubyContext context, SourceSection section, RubyNode receiver, MethodDefinitionNode method, boolean isDefs) {
         super(context, section);
+        this.isDefs = isDefs;
         this.receiver = receiver;
         this.methodNode = method;
         this.singletonClassNode = SingletonClassNodeGen.create(context, section, null);
@@ -65,37 +65,13 @@ public DynamicObject execute(VirtualFrame frame) {
         return getSymbol(method.getName());
     }
 
-    private static Visibility getVisibility(Frame frame, String name) {
-        if (ModuleOperations.isMethodPrivateFromName(name)) {
+    private Visibility getVisibility(Frame frame, String name) {
+        if (isDefs) {
+            return Visibility.PUBLIC;
+        } else if (ModuleOperations.isMethodPrivateFromName(name)) {
             return Visibility.PRIVATE;
         } else {
-            return getVisibility(frame);
-        }
-    }
-
-    public static Visibility getVisibility(Frame frame) {
-        while (frame != null) {
-            Visibility visibility = findVisibility(frame);
-            if (visibility != null) {
-                return visibility;
-            }
-            frame = RubyArguments.getDeclarationFrame(frame.getArguments());
-        }
-
-        throw new UnsupportedOperationException("No declaration frame with visibility found");
-    }
-
-    private static Visibility findVisibility(Frame frame) {
-        FrameSlot slot = frame.getFrameDescriptor().findFrameSlot(ModuleNodes.VISIBILITY_FRAME_SLOT_ID);
-        if (slot == null) {
-            return null;
-        } else {
-            Object visibilityObject = frame.getValue(slot);
-            if (visibilityObject instanceof Visibility) {
-                return (Visibility) visibilityObject;
-            } else {
-                return Visibility.PUBLIC;
-            }
+            return DeclarationContext.findVisibility(frame);
         }
     }
 }
diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/methods/DeclarationContext.java b/truffle/src/main/java/org/jruby/truffle/nodes/methods/DeclarationContext.java
@@ -14,6 +14,7 @@
 import org.jruby.truffle.runtime.RubyArguments;
 import org.jruby.truffle.runtime.RubyContext;
 
+import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.object.DynamicObject;
 
@@ -41,15 +42,28 @@ public DeclarationContext(Visibility visibility, DefaultDefinee defaultDefinee)
         this.defaultDefinee = defaultDefinee;
     }
 
-    public Visibility getVisibility() {
-        return visibility;
+    private static Frame lookupVisibility(Frame frame) {
+        while (frame != null) {
+            final Visibility visibility = RubyArguments.getDeclarationContext(frame.getArguments()).visibility;
+            if (visibility != null) {
+                return frame;
+            }
+            frame = RubyArguments.getDeclarationFrame(frame.getArguments());
+        }
+        throw new UnsupportedOperationException("No declaration frame with visibility found");
+    }
+
+    public static Visibility findVisibility(Frame frame) {
+        final Frame visibilityFrame = lookupVisibility(frame);
+        return RubyArguments.getDeclarationContext(visibilityFrame.getArguments()).visibility;
     }
 
-    public DeclarationContext withVisibility(Visibility visibility) {
-        if (visibility == this.visibility) {
-            return this;
-        } else {
-            return new DeclarationContext(visibility, this.defaultDefinee);
+    public static void changeVisibility(Frame frame, Visibility newVisibility) {
+        final Frame visibilityFrame = lookupVisibility(frame);
+        final DeclarationContext oldDeclarationContext = RubyArguments.getDeclarationContext(visibilityFrame.getArguments());
+        if (newVisibility != oldDeclarationContext.visibility) {
+            final DeclarationContext newDeclarationContext = new DeclarationContext(newVisibility, oldDeclarationContext.defaultDefinee);
+            RubyArguments.setDeclarationContext(visibilityFrame.getArguments(), newDeclarationContext);
         }
     }
 
@@ -68,9 +82,9 @@ public DynamicObject getModuleToDefineMethods(VirtualFrame frame, RubyContext co
     }
 
     public static final DeclarationContext MODULE = new DeclarationContext(Visibility.PUBLIC, DefaultDefinee.LEXICAL_SCOPE);
-    public static final DeclarationContext METHOD = MODULE;
-    public static final DeclarationContext BLOCK = METHOD;
-    public static final DeclarationContext TOP_LEVEL = METHOD;
+    public static final DeclarationContext METHOD = new DeclarationContext(null, DefaultDefinee.LEXICAL_SCOPE);
+    public static final DeclarationContext BLOCK = new DeclarationContext(null, DefaultDefinee.LEXICAL_SCOPE);
+    public static final DeclarationContext TOP_LEVEL = new DeclarationContext(Visibility.PRIVATE, DefaultDefinee.LEXICAL_SCOPE);
     public static final DeclarationContext INSTANCE_EVAL = new DeclarationContext(Visibility.PUBLIC, DefaultDefinee.SINGLETON_CLASS);
     public static final DeclarationContext CLASS_EVAL = new DeclarationContext(Visibility.PUBLIC, DefaultDefinee.SELF);
 

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/methods/SetMethodDeclarationContext.java b/truffle/src/main/java/org/jruby/truffle/nodes/methods/SetMethodDeclarationContext.java
-Original file line number
+Diff line change
@@ @@ -16,6 +16,9 @@ def foo @@
         end
         private
+        def self.public_defs_method; 0; end
         class C
           def baz
             "baz"
@@ @@ -27,7 +30,6 @@ def public_class_method1; 1; end @@
           private
           def private_class_method1; 1; end
         end
-        def self.public_class_method2; 2; end
         def bar
           "bar"
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		fails:Module#private without arguments does not affect method definitions when itself is inside an eval and method definitions are outside
		fails:Module#private without arguments within a closure sets the visibility outside the closure
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		fails:Module#protected without arguments does not affect method definitions when itself is inside an eval and method definitions are outside
		fails:Module#protected without arguments within a closure sets the visibility outside the closure