[Truffle] Refactor a good part of Module concerning visibility.

* @coremethod with "private" names are now automatically private. * At last, proper implementation of Module.{public_,private_,protected_,}methods.
jruby · Apr 21, 2015 · 06c53b9 · 06c53b9
1 parent e7c7beb
commit 06c53b9
Show file tree

Hide file tree

Showing 22 changed files with 223 additions and 156 deletions.
diff --git a/spec/truffle/tags/core/module/extend_object_tags.txt b/spec/truffle/tags/core/module/extend_object_tags.txt
diff --git a/spec/truffle/tags/core/module/module_function_tags.txt b/spec/truffle/tags/core/module/module_function_tags.txt
@@ -1,6 +1,2 @@
-fails:Module#module_function is a private method
-fails:Module#module_function on Class raises a TypeError if calling after rebinded to Class
-fails:Module#module_function with specific method names tries to convert the given names to strings using to_str
-fails:Module#module_function with specific method names raises a TypeError when the given names can't be converted to string using to_str
 fails:Module#module_function as a toggle (no arguments) in a Module body affects evaled method definitions also even when outside the eval itself
 fails:Module#module_function as a toggle (no arguments) in a Module body functions normally if both toggle and definitions inside a eval
diff --git a/spec/truffle/tags/core/module/private_tags.txt b/spec/truffle/tags/core/module/private_tags.txt
@@ -1,6 +1,3 @@
-fails:Module#private is a private method
-fails:Module#private makes a public Object instance method private in Kernel
-fails:Module#private raises a NameError when given an undefined name
 fails:Module#private without arguments sets visibility to following method definitions
 fails:Module#private without arguments stops setting visibility if the body encounters other visibility setters without arguments
 fails:Module#private without arguments continues setting visibility if the body encounters other visibility setters with arguments

diff --git a/spec/truffle/tags/core/module/protected_tags.txt b/spec/truffle/tags/core/module/protected_tags.txt
@@ -1,6 +1,3 @@
-fails:Module#protected is a private method
-fails:Module#protected makes a public Object instance method protected in Kernel
-fails:Module#protected raises a NameError when given an undefined name
 fails:Module#protected without arguments sets visibility to following method definitions
 fails:Module#protected without arguments stops setting visibility if the body encounters other visibility setters without arguments
 fails:Module#protected without arguments continues setting visibility if the body encounters other visibility setters with arguments

diff --git a/spec/truffle/tags/core/module/public_tags.txt b/spec/truffle/tags/core/module/public_tags.txt
@@ -1,7 +1,4 @@
-fails:Module#public is a private method
 fails:Module#public on a superclass method calls the redefined method
-fails:Module#public makes a private Object instance method public in Kernel
-fails:Module#public raises a NameError when given an undefined name
 fails:Module#public without arguments stops setting visibility if the body encounters other visibility setters without arguments
 fails:Module#public without arguments does not affect method definitions when itself is inside an eval and method definitions are outside
 fails:Module#public without arguments affects evaled method definitions when itself is outside the eval

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/coerce/SymbolOrToStrNode.java b/truffle/src/main/java/org/jruby/truffle/nodes/coerce/SymbolOrToStrNode.java
@@ -39,6 +39,8 @@ public SymbolOrToStrNode(SymbolOrToStrNode prev) {
         toStr = prev.toStr;
     }
 
+    public abstract String executeToJavaString(VirtualFrame frame, Object object);
+
     @Specialization
     public String coerceRubySymbol(RubySymbol symbol) {
         return symbol.toString();

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/ArrayNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/ArrayNodes.java
@@ -1936,11 +1936,8 @@ public RubyArray initialize(RubyArray array, RubyArray copy, UndefinedPlaceholde
 
     }
 
-    @CoreMethod(names = "initialize_copy", visibility = Visibility.PRIVATE, required = 1, raiseIfFrozenSelf = true)
-    @NodeChildren({
-        @NodeChild(value = "self"),
-        @NodeChild(value = "from")
-    })
+    @CoreMethod(names = "initialize_copy", required = 1, raiseIfFrozenSelf = true)
+    @NodeChildren({ @NodeChild(value = "self"), @NodeChild(value = "from") })
     @ImportGuards(ArrayGuards.class)
     public abstract static class InitializeCopyNode extends RubyNode {
         // TODO(cs): what about allocationSite ?

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/BasicObjectNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/BasicObjectNodes.java
@@ -174,7 +174,7 @@ protected boolean notSameClass(Object a, Object b) {
 
     }
 
-    @CoreMethod(names = "initialize", needsSelf = false, visibility = Visibility.PRIVATE)
+    @CoreMethod(names = "initialize", needsSelf = false)
     public abstract static class InitializeNode extends CoreMethodNode {
 
         public InitializeNode(RubyContext context, SourceSection sourceSection) {

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
@@ -27,7 +27,7 @@
 @CoreClass(name = "Binding")
 public abstract class BindingNodes {
 
-    @CoreMethod(names = "initialize_copy", visibility = Visibility.PRIVATE, required = 1)
+    @CoreMethod(names = "initialize_copy", required = 1)
     public abstract static class InitializeCopyNode extends CoreMethodNode {
 
         public InitializeCopyNode(RubyContext context, SourceSection sourceSection) {

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/CoreMethodNodeManager.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/CoreMethodNodeManager.java
@@ -116,14 +116,19 @@ private static void addMethod(RubyClass rubyObjectClass, MethodDetails methodDet
         }
     }
 
-    private static void addMethod(RubyModule module, RubyRootNode rootNode, List<String> names, Visibility visibility) {
+    private static void addMethod(RubyModule module, RubyRootNode rootNode, List<String> names, final Visibility originalVisibility) {
         for (String name : names) {
             final RubyRootNode rootNodeCopy = NodeUtil.cloneNode(rootNode);
 
+            Visibility visibility = originalVisibility;
+            if (ModuleOperations.isMethodPrivateFromName(name)) {
+                visibility = Visibility.PRIVATE;
+            }
+
             final InternalMethod method = new InternalMethod(rootNodeCopy.getSharedMethodInfo(), name, module, visibility, false,
                     Truffle.getRuntime().createCallTarget(rootNodeCopy), null);
 
-            module.addMethod(null, method.withVisibility(visibility).withNewName(name));
+            module.addMethod(null, method.withVisibility(visibility).withName(name));
         }
     }
 

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
@@ -573,11 +573,7 @@ public Object eval(RubyString source, RubyBinding binding, RubyString filename,
 
         @Specialization(guards = "!isRubyBinding(binding)")
         public Object eval(RubyString source, RubyBasicObject badBinding, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
-            throw new RaiseException(
-                    getContext().getCoreLibrary().typeError(
-                            String.format("wrong argument type %s (expected binding)",
-                                    badBinding.getLogicalClass().getName()),
-                            this));
+            throw new RaiseException(getContext().getCoreLibrary().typeErrorWrongArgumentType(badBinding, "binding", this));
         }
     }
 
@@ -870,7 +866,7 @@ public int hash(RubyBasicObject self) {
 
     }
 
-    @CoreMethod(names = "initialize_copy", visibility = Visibility.PRIVATE, required = 1)
+    @CoreMethod(names = "initialize_copy", required = 1)
     public abstract static class InitializeCopyNode extends CoreMethodNode {
 
         public InitializeCopyNode(RubyContext context, SourceSection sourceSection) {
@@ -895,7 +891,7 @@ public Object initializeCopy(RubyBasicObject self, RubyBasicObject from) {
 
     }
 
-    @CoreMethod(names = {"initialize_dup", "initialize_clone"}, visibility = Visibility.PRIVATE, required = 1)
+    @CoreMethod(names = { "initialize_dup", "initialize_clone" }, required = 1)
     public abstract static class InitializeDupCloneNode extends CoreMethodNode {
 
         @Child private CallDispatchHeadNode initializeCopyNode;
@@ -1603,7 +1599,7 @@ public boolean doesRespondTo(VirtualFrame frame, Object object, RubySymbol name,
         }
     }
 
-    @CoreMethod(names = "respond_to_missing?", required = 1, optional = 1, visibility = Visibility.PRIVATE)
+    @CoreMethod(names = "respond_to_missing?", required = 1, optional = 1)
     public abstract static class RespondToMissingNode extends CoreMethodNode {
 
         public RespondToMissingNode(RubyContext context, SourceSection sourceSection) {