Fix Module#define_method and apply module_function visibility

Author: andrykonchin

CHANGELOG.md

@@ -9,6 +9,7 @@ Bug fixes:
 * Fix class lookup after an object's class has been replaced by `IO#reopen` (@itarato, @eregon).
 * Fix `Marshal.load` and raise `ArgumentError` when dump is broken and is too short (#3108, @andrykonchin).
 * Fix `super` method lookup for unbounded attached methods (#3131,  @itarato).
+* Fix `Module#define_method(name, Method)` to respect `module_function` visibility (#3181, @andrykonchin).
 
 Compatibility:
 

spec/ruby/core/module/module_function_spec.rb

@@ -155,6 +155,53 @@ def foo
 
     m.foo.should == ["m", "super_m"]
   end
+
+  context "methods created with define_method" do
+    context "passed a block" do
+      it "creates duplicates of the given instance methods" do
+        m = Module.new do
+          define_method :test1 do; end
+          module_function :test1
+        end
+
+        m.respond_to?(:test1).should == true
+      end
+    end
+
+    context "passed a method" do
+      it "creates duplicates of the given instance methods" do
+        module_with_method = Module.new do
+          def test1; end
+        end
+
+        c = Class.new do
+          extend module_with_method
+        end
+
+        m = Module.new do
+          define_method :test2, c.method(:test1)
+          module_function :test2
+        end
+
+        m.respond_to?(:test2).should == true
+      end
+    end
+
+    context "passed an unbound method" do
+      it "creates duplicates of the given instance methods" do
+        module_with_method = Module.new do
+          def test1; end
+        end
+
+        m = Module.new do
+          define_method :test2, module_with_method.instance_method(:test1)
+          module_function :test2
+        end
+
+        m.respond_to?(:test2).should == true
+      end
+    end
+  end
 end
 
 describe "Module#module_function as a toggle (no arguments) in a Module body" do
@@ -282,4 +329,51 @@ def test2() end
     m.respond_to?(:test1).should == true
     m.respond_to?(:test2).should == true
   end
+
+  context "methods are defined with define_method" do
+    context "passed a block" do
+      it "makes any subsequently defined methods module functions with the normal semantics" do
+        m = Module.new do
+          module_function
+          define_method :test1 do; end
+        end
+
+        m.respond_to?(:test1).should == true
+      end
+    end
+
+    context "passed a method" do
+      it "makes any subsequently defined methods module functions with the normal semantics" do
+        module_with_method = Module.new do
+          def test1; end
+        end
+
+        c = Class.new do
+          extend module_with_method
+        end
+
+        m = Module.new do
+          module_function
+          define_method :test2, c.method(:test1)
+        end
+
+        m.respond_to?(:test2).should == true
+      end
+    end
+
+    context "passed an unbound method" do
+      it "makes any subsequently defined methods module functions with the normal semantics" do
+        module_with_method = Module.new do
+          def test1; end
+        end
+
+        m = Module.new do
+          module_function
+          define_method :test2, module_with_method.instance_method(:test1)
+        end
+
+        m.respond_to?(:test2).should == true
+      end
+    end
+  end
 end

src/main/java/org/truffleruby/core/module/ModuleNodes.java

@@ -1240,7 +1240,8 @@ protected RubySymbol defineMethodWithMethod(
             final String name = nameToJavaStringNode.execute(this, RubyArguments.getArgument(rubyArgs, 0));
             final Object method = RubyArguments.getArgument(rubyArgs, 1);
 
-            return addMethod(module, name, (RubyMethod) method);
+            needCallerFrame(callerFrame, target);
+            return addMethod(module, name, (RubyMethod) method, callerFrame.materialize());
         }
 
         @Specialization(guards = { "isMethodParameterProvided(rubyArgs)", "isRubyProc(getArgument(rubyArgs, 1))" })
@@ -1294,7 +1295,8 @@ protected RubySymbol defineMethodWithoutMethodAndBlock(
         }
 
         @TruffleBoundary
-        private RubySymbol addMethod(RubyModule module, String name, RubyMethod method) {
+        private RubySymbol addMethod(RubyModule module, String name, RubyMethod method,
+                MaterializedFrame callerFrame) {
             final InternalMethod internalMethod = method.method;
 
             if (!ModuleOperations.canBindMethodTo(internalMethod, module)) {
@@ -1310,8 +1312,7 @@ private RubySymbol addMethod(RubyModule module, String name, RubyMethod method)
                 }
             }
 
-            module.fields.addMethod(getContext(), this, internalMethod.withName(name));
-            return getSymbol(name);
+            return addInternalMethod(module, name, internalMethod, callerFrame);
         }
 
         @TruffleBoundary