Merge pull request #3617 from rubinius/clean_up_define_method

Clean up define method
rubinius · Feb 8, 2016 · e2d1437 · e2d1437
2 parents d436251 + eb465ca
commit e2d1437
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 52 deletions.
diff --git a/core/block_environment.rb b/core/block_environment.rb
@@ -102,6 +102,10 @@ def file
       def defined_line
         @block_env.line
       end
+
+      def scope
+        @block_env.scope
+      end
     end
 
     def from_proc?

diff --git a/core/delegated_method.rb b/core/delegated_method.rb
@@ -31,5 +31,9 @@ def parameters
     def source_location
       @receiver.source_location
     end
+
+    def scope
+      nil
+    end
   end
 end
diff --git a/core/method.rb b/core/method.rb
@@ -172,6 +172,23 @@ def super_method
     return nil
   end
 
+  def for_define_method(name, klass, callable_proc = nil)
+    Rubinius::Type.bindable_method? self.defined_in, klass
+
+    scope = @executable.scope
+
+    if @executable.is_a? Rubinius::DelegatedMethod
+      code = @executable
+    else
+      if callable_proc
+        code = Rubinius::DelegatedMethod.new(name, :call, callable_proc, false)
+      else
+        code = Rubinius::DelegatedMethod.new(name, :call_on_instance, self.unbind, true)
+      end
+    end
+
+    [code, scope]
+  end
 end
 
 ##
@@ -316,4 +333,22 @@ def super_method
 
     return nil
   end
+
+  def for_define_method(name, klass, callable_proc = nil)
+    Rubinius::Type.bindable_method? self.defined_in, klass
+
+    scope = @executable.scope
+
+    if @executable.is_a? Rubinius::DelegatedMethod
+      code = @executable
+    else
+      if callable_proc
+        code = Rubinius::DelegatedMethod.new(name, :call, callable_proc, false)
+      else
+        code = Rubinius::DelegatedMethod.new(name, :call_on_instance, self, true)
+      end
+    end
+
+    [code, scope]
+  end
 end
diff --git a/core/module.rb b/core/module.rb
@@ -442,58 +442,8 @@ def define_method(name, meth = undefined, &prc)
 
     name = Rubinius::Type.coerce_to_symbol name
 
-    case meth
-    when Proc
-      if meth.ruby_method
-        code = Rubinius::DelegatedMethod.new(name, :call, meth, false)
-        if meth.ruby_method.executable.kind_of? Rubinius::CompiledCode
-          scope = meth.ruby_method.executable.scope
-        else
-          scope = nil
-        end
-      else
-        be = meth.block.dup
-        be.change_name name
-        code = Rubinius::BlockEnvironment::AsMethod.new(be)
-        meth = meth.dup
-        meth.lambda_style!
-        scope = meth.block.scope
-      end
-    when Method
-      Rubinius::Type.bindable_method? meth.defined_in, self.class
-
-      exec = meth.executable
-      # We see through delegated methods because code creates these crazy calls
-      # to define_method over and over again and if we don't check, we create
-      # a huge delegated method chain. So instead, just see through them at one
-      # level always.
-      if exec.kind_of? Rubinius::DelegatedMethod
-        code = exec
-        scope = nil
-      else
-        code = Rubinius::DelegatedMethod.new(name, :call_on_instance, meth.unbind, true)
-        if exec.kind_of? Rubinius::CompiledCode
-          scope = exec.scope
-        else
-          scope = nil
-        end
-      end
-    when UnboundMethod
-      Rubinius::Type.bindable_method? meth.defined_in, self.class
-
-      exec = meth.executable
-      # Same reasoning as above.
-      if exec.kind_of? Rubinius::DelegatedMethod
-        code = exec
-        scope = nil
-      else
-        code = Rubinius::DelegatedMethod.new(name, :call_on_instance, meth, true)
-        if exec.kind_of? Rubinius::CompiledCode
-          scope = exec.scope
-        else
-          scope = nil
-        end
-      end
+    if meth.respond_to?(:for_define_method)
+      code, scope = meth.for_define_method(name, self.class)
     else
       raise TypeError, "wrong argument type #{meth.class} (expected Proc/Method)"
     end

diff --git a/core/native_method.rb b/core/native_method.rb
@@ -78,5 +78,9 @@ def defined_line
     def active_path
       @file
     end
+
+    def scope
+      nil
+    end
   end
 end
diff --git a/core/proc.rb b/core/proc.rb
@@ -244,6 +244,23 @@ def dup
     copy
   end
 
+  def for_define_method(name, klass)
+    if @ruby_method
+      code, scope = @ruby_method.for_define_method(name, klass, self)
+    else
+      be = @block.dup
+      be.change_name name
+
+      duped_proc = self.dup
+      duped_proc.lambda_style!
+
+      code  = Rubinius::BlockEnvironment::AsMethod.new(be)
+      scope = duped_proc.block.scope
+    end
+
+    [code, scope]
+  end
+
   def self.from_method(meth)
     if meth.kind_of? Method
       return __from_method__(meth)

diff --git a/spec/ruby/core/module/define_method_spec.rb b/spec/ruby/core/module/define_method_spec.rb
@@ -180,6 +180,26 @@ def inspect_data
     lambda{o.other_inspect}.should raise_error(NoMethodError)
   end
 
+  it "accepts a proc from a method" do
+    class ProcFromMethod
+      attr_accessor :data
+      def cool_method
+        "data is #{@data}"
+      end
+    end
+
+    object1 = ProcFromMethod.new
+    object1.data = :foo
+
+    method_proc = object1.method(:cool_method).to_proc
+    klass = Class.new(ProcFromMethod)
+    klass.send(:define_method, :other_cool_method, &method_proc)
+
+    object2 = klass.new
+    object2.data = :bar
+    object2.other_cool_method.should == "data is foo"
+  end
+
   it "maintains the Proc's scope" do
     class DefineMethodByProcClass
       in_scope = true