From: "jonathanhefner (Jonathan Hefner)" <noreply@...>
Date: 2022-10-11T18:09:58+00:00
Subject: [ruby-core:110257] [Ruby master Bug#19047] DelegateClass displays "method redefined" warning when overriding methods

Issue #19047 has been updated by jonathanhefner (Jonathan Hefner).


Indeed, `alias_method` is another possibility!  And also [Jean's suggestion](https://github.com/rails/rails/pull/46189#discussion_r988685084) of defining the delegator methods in a module.

Unfortunately, my suggestion of evaluating the block in a module does not work for metaprogramming within the block, such as [in this test](https://github.com/ruby/delegate/blob/df2283b8d8874446086b80355c586397f1b48d53/test/test_delegate.rb#L27).

I had not really dug into `DelegateClass` to understand why the `super` in `Delegate1` still works after redefining `foo`.  I see now that it's because the class [is a subclass of `Delegator`](https://github.com/ruby/delegate/blob/df2283b8d8874446086b80355c586397f1b48d53/lib/delegate.rb#L395), which also [responds to `foo` via `method_missing`](https://github.com/ruby/delegate/blob/df2283b8d8874446086b80355c586397f1b48d53/lib/delegate.rb#L82-L93).  Therefore, redefining `foo` causes `super` to fall back to a slower execution path.

So I benchmarked the solutions with this script:

```ruby
# frozen_string_literal: true
require "benchmark/ips"
$LOAD_PATH.prepend(".../delegate/lib")
require "delegate"

Base = Class.new { def foo; end }
Overridden = DelegateClass(Base) { def foo; super; end }
overridden = Overridden.new(Base.new)

Benchmark.ips do |x|
  x.report("overridden") { overridden.foo }
end
```

With this `alias_method` patch:

```diff
index 70d4e4a..af95c86 100644
--- a/lib/delegate.rb
+++ b/lib/delegate.rb
@@ -412,10 +412,12 @@ def DelegateClass(superclass, &block)
     end
     protected_instance_methods.each do |method|
       define_method(method, Delegator.delegating_block(method))
+      alias_method(method, method)
       protected method
     end
     public_instance_methods.each do |method|
       define_method(method, Delegator.delegating_block(method))
+      alias_method(method, method)
     end
   end
   klass.define_singleton_method :public_instance_methods do |all=true|
```

the result is:

```
Warming up --------------------------------------
          overridden    76.674k i/100ms
Calculating -------------------------------------
          overridden    765.489k (�� 0.9%) i/s -      3.834M in   5.008559s
```

With this delegator methods module patch:

```diff
index 70d4e4a..4311bb5 100644
--- a/lib/delegate.rb
+++ b/lib/delegate.rb
@@ -410,6 +410,8 @@ def DelegateClass(superclass, &block)
       __raise__ ::ArgumentError, "cannot delegate to self" if self.equal?(obj)
       @delegate_dc_obj = obj
     end
+  end
+  klass.include(Module.new do
     protected_instance_methods.each do |method|
       define_method(method, Delegator.delegating_block(method))
       protected method
@@ -417,7 +419,7 @@ def DelegateClass(superclass, &block)
     public_instance_methods.each do |method|
       define_method(method, Delegator.delegating_block(method))
     end
-  end
+  end)
   klass.define_singleton_method :public_instance_methods do |all=true|
     super(all) | superclass.public_instance_methods
   end
```

the result is:

```
Warming up --------------------------------------
          overridden   183.938k i/100ms
Calculating -------------------------------------
          overridden      1.830M (�� 0.9%) i/s -      9.197M in   5.026683s
```

Comparison: the `alias_method` solution is 2.39x slower than the delegator methods module solution.

Is there another reason to prefer the `alias_method` solution?


----------------------------------------
Bug #19047: DelegateClass displays "method redefined" warning when overriding methods
https://bugs.ruby-lang.org/issues/19047#change-99545

* Author: jonathanhefner (Jonathan Hefner)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.1.2p20
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
Perhaps this is not a bug, but it does seem unexpected.

When creating a `DelegateClass` class without an intervening ancestor, overriding a method displays "method redefined" warning:

```ruby
Base = Class.new do
  def foo
    "foo"
  end
end

Delegate1 = DelegateClass(Base) do
  def foo
    super + "1"
  end
end
# warning: method redefined; discarding old foo

Delegate2 = Class.new(DelegateClass(Base)) do
  def foo
    super + "2"
  end
end
# no warning

Delegate1.new(Base.new).foo
# => "foo1"

Delegate2.new(Base.new).foo
# => "foo2"
```

One possible solution would be to evaluate the `DelegateClass` block in a separate module, and prepend that module to the returned class.

Another possible solution would be to silence warnings around [when the block is evaluated](https://github.com/ruby/delegate/blob/df2283b8d8874446086b80355c586397f1b48d53/lib/delegate.rb#L442).

I would be happy to submit a PR to https://github.com/ruby/delegate if this is something we want to address.




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>