Bug #22273: Aliasing doesn't interact well with Module#prepend

• Status: Open
• Assignee: jeremyevans0 (Jeremy Evans)
• Target version: -
• Backport: 3.3: DONTNEED, 3.4: DONTNEED, 4.0: DONTNEED

[luke-gru (Luke Gruber)]

Currently, aliasing doesn't interact well with Module#prepend in my opinion.

Example

module Kernel
  prepend(Module.new do
    def require(feature)
      puts "requiring feature (prepend): #{feature}"
      super
    end
  end)
end

module Kernel
  alias original_require require
  def require(feature)
    puts "requiring feature (alias): #{feature}"
    original_require(feature)
  end
end

require "set"

This produces this behavior:

requiring feature (prepend): set
requiring feature (alias): set
requiring feature (prepend): set
../ruby/test.rb:5:in 'require': super: no superclass method 'require' for main (NoMethodError)

I would expect this behavior:

requiring feature (prepend): set
requiring feature (alias): set
# Then, the original require would succeed

This has caused issues such as 22263 and has confused gem authors. In the second link, the ignored alias was due to this bug which has recently been fixed.

Bug?

As far as I know this is intentional behavior introduced in Ruby 2.0 here. There are even tests and specs that codify this behavior such as test_prepend_super_in_alias and prepend_spec.rb.

Even though it's intentional, I don't believe it's well thought out. I'm interested in hearing arguments for and against the current behavior (with code examples, preferably).

[jhawthorn (John Hawthorn)]

• Related to Bug #7842: An alias of a "prepend"ed method skips the original method when calling super added

[jhawthorn (John Hawthorn)]

• Related to Bug #22263: NoMethodError: super: no superclass method 'require' for main — raised in a forked child process after a Ractor has exited in the parent (bisected to c59c4d717a) added

[luke-gru (Luke Gruber)]

• Description updated (diff)

[jeremyevans0 (Jeremy Evans)]

As I mentioned in https://bugs.ruby-lang.org/issues/22263#note-5, I think a module should only be able to alias methods in ancestor modules, it should not be able to alias methods in descendant modules, as I don't think the semantics make sense. As prepend results in a descendant module and not an ancestor module from the perspective of the the module's own methods in the lookup chain, I don't think a module should be able to alias a method in a module prepended to it.

For example, if you do:

module M
  def m = 1
end
class C
  prepend M
  alias m2 m
end

I think the alias m2 m should raise NameError.

When you allow a module to alias a method in a descendant module, you appear to end up with a case where super calls can to go to a descendant instead of an ancestor:

module M
  def m = [M, :m, *super]
end
module N
  def m2 = [N, :m2, *super]
end
class Object
  def m = [Object]
end
class C
  prepend N
  alias m m2
  prepend M
  alias m2 m
  def m = [C, :m, *super]
end
C.ancestors
# => [M, N, C, Object, Kernel, BasicObject]
C.new.m2
# => [N, :m2, M, :m, C, :m, Object]
m = C.new.method(:m2)
# => #<Method: C(N)#m2() -:5>
m = m.super_method
# => #<Method: C(M)#m2(m)() -:2>
m = m.super_method
# => #<Method: C#m() -:15>
m = m.super_method
# => #<Method: Object#m() -:8>
m = m.super_method
# => nil

[shugo (Shugo Maeda)]

I agree with Jeremy, and would like to point out that Ruby already has exactly this semantics for refinements.

If the prepend in the example is replaced with a refinement, we get the expected behavior:

using Module.new {
  refine Kernel do
    def require(feature)
      puts "requiring feature (refine): #{feature}"
      super
    end
  end
}

module Kernel
  alias original_require require
  def require(feature)
    puts "requiring feature (alias): #{feature}"
    original_require(feature)
  end
end

require "set"

requiring feature (refine): set
requiring feature (alias): set
# and the original require succeeds

alias never sees the refinement layer.

If the method exists only in the refinement, alias raises NameError:

class C; end
using Module.new { refine(C) { def m = :refined } }
class C
  alias m2 m  #=> NameError: undefined method 'm' for class 'C'
end

This is precisely what Jeremy proposes for prepend: a module can alias only its own methods and its ancestors' methods, and aliasing a method that exists only in a prepended module raises NameError. Since a refinement and a prepended module play the same role (a layer in front of the class's own methods, whose super reaches the class's own method), I think prepend should behave the same way as refinements here.

Another argument: the current behavior is already inconsistent between classes and modules. The same pattern as in the ticket applied to a class does not raise NoMethodError but loops forever:

class A; def m = [:A]; end
class B < A
  prepend(Module.new { def m = [:P, *super] })
  alias orig m
  def m = [:new, *orig]
end
B.new.m  #=> SystemStackError

With the module version, the same code raises NoMethodError instead. This comes from how the defined class of the aliased method is resolved for super (rb_find_defined_class_by_owner). If alias looked up the method from the origin class, both cases would simply alias the class's own method, and the difference would disappear.

One thing to be careful about when implementing this: if the target is a module and the method is not found, rb_alias falls back to searching from Object. So even if alias looked up the method from the origin class, a method that exists only in a module prepended to Kernel would still be found through Object's ancestors when aliased in module Kernel.

[Eregon (Benoit Daloze)]

What if one actually wants to alias a method from a prepended module, how should they do it?

Maybe

define_method(:my_alias, instance_method(:original))

?
That gives the same result.

---

My general thinking is alias and alias_method should do exactly the same as define_method(alias_name, instance_method(original_name)).
That would be so much easier to understand and much simpler semantically (ZSUPER aliases add so many complications).
Though in this issue it seems they are equivalent for this reproduction.

---

Mixing prepend (especially on Kernel) and alias works poorly in general, I think one should use one or the other.
Prepending on Kernel is very disruptive as it changes the ancestors of all classes (except direct subclasses of BasicObject).

[Eregon (Benoit Daloze)]

In short, alias and alias_method do a lookup similar to instance_method.
While changing that would fix this specific case it could break others.
And I guess most agree we shouldn't change how instance_method looks things up (i.e. it should find prepended methods), so if we'd change alias lookup we'd make it inconsistent.

[jeremyevans0 (Jeremy Evans)]

Eregon (Benoit Daloze) wrote in #note-6:

What if one actually wants to alias a method from a prepended module, how should they do it?

Maybe

define_method(:my_alias, instance_method(:original))

?
That gives the same result.

Actually, it doesn't, for the example I gave above:

module M
  def m = [M, :m, *super]
end
module N
  def m2 = [N, :m2, *super]
end
class Object
  def m = [Object]
end
class C
  prepend N
  define_method(:m, instance_method(:m2))
  prepend M
  define_method(:m2, instance_method(:m))
  def m = [C, :m, *super]
end
C.ancestors
# => [M, N, C, Object, Kernel, BasicObject]
C.new.m2
# => [N, :m2, M, :m, Object]
m = C.new.method(:m2)
# => #<Method: C(N)#m2() -:5>
m = m.super_method
# => #<Method: C#m2(m)() (irb):2>
m = m.super_method
# => #<Method: Object#m() -:8>
m = m.super_method
# => nil

Note that it in this case, the super calls do not reverse the ancestry, in terms of the method owner. For the alias case, it goes N -> M -> C, even though the ancestry is M -> N -> C. With define_method, it goes N -> C, which is consistent with the ancestry.

To answer your question about how to alias a method in a prepended module, you would define a method that calls the method you want to alias (or use define_method as shown above) instead of using alias/alias_method. All of these approaches have different semantics (as shown above), but for most cases, either new method that calls original method or define_method with original method should work.

My general thinking is alias and alias_method should do exactly the same as define_method(alias_name, instance_method(original_name)).
That would be so much easier to understand and much simpler semantically (ZSUPER aliases add so many complications).

Though in this issue it seems they are equivalent for this reproduction.

I agree that would be simpler, but they are not equivalent. For one, define_method doesn't even require the original method be from the current lookup hierarchy, you can use an instance method from a module the class doesn't include/prepend, whereas alias/alias_method require the method be defined in the lookup hierarchy.

Mixing prepend (especially on Kernel) and alias works poorly in general, I think one should use one or the other.

Agreed that it works poorly. More importantly to me is this issue with super allows behavior that I don't think should be allowed.

In short, alias and alias_method do a lookup similar to instance_method.
While changing that would fix this specific case it could break others.
And I guess most agree we shouldn't change how instance_method looks things up (i.e. it should find prepended methods), so if we'd change alias lookup we'd make it inconsistent.

As shown above, alias and define_method with instance_method operate differently and define_method with instance_method does not have the same issue with super. As the behavior already differs between the two cases, I don't think they need to be consistent.

In terms of your idea that alias/alias_method operate like define_method(alias_name, instance_method(original_name)), that would be a different breaking change.

[matz (Yukihiro Matsumoto)]

I accept Jeremy's proposal. alias should look up the method from the origin class, so it only sees the methods of the class/module itself and its ancestors. Aliasing a method that exists only in a prepended module should raise NameError.

This reverses my decision in #7842. alias is an operation on definitions like def and remove_method, not a method lookup from outside. Prepended modules are layers in front of those definitions, so alias should not capture them. This is also the same principle as #22276.

If you want an alias that goes through prepended modules, define a forwarding method (e.g. def a(...) = b(...)) instead.

I leave the migration path to the implementer.

Matz.

[jeremyevans0 (Jeremy Evans)]

• Backport changed from 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN to 3.3: DONTNEED, 3.4: DONTNEED, 4.0: DONTNEED

I submitted a pull request to add the initial deprecation warning: https://github.com/ruby/ruby/pull/18807

I think it makes sense to use the same schedule as #22276:

4.1: Verbose-mode deprecation warning
4.2: Non-verbose-mode deprecation warning
4.3: Start original method lookup at the origin

[fxn (Xavier Noria)]

Besides the fundamental semantics of aliasing + prepending, I would like to stress that we have here yet another use case of Kernel#require decoration.

I don't want to hijack the ticket, but would like to leverage it to ask you: Don't you think all these use cases may hint the language would benefit from providing first-class support for hooking into the require workflow?

• Status changed from Open to Closed

Applied in changeset git|1c13d07cfca130ae3c0631d24ef21680099c02a7.

---

Emit deprecation warning for aliasing method in prepended module

Aliasing a method in a prepended module can result in a super
call going into a descendant instead of an ancestor. Removal plan:

4.1: Deprecation warning
4.2: Warning even in non-verbose mode
4.3: Removal (target method lookup starts at origin class)

Fixes [Bug #22273]

[jeremyevans0 (Jeremy Evans)]

• Status changed from Closed to Open
• Assignee set to jeremyevans0 (Jeremy Evans)

Reopening, since this should stay open until removal occurs.