Consider ZSUPER methods as separate methods for Method/UnboundMethod - #5802
Consider ZSUPER methods as separate methods for Method/UnboundMethod#5802jeremyevans wants to merge 1 commit into
Conversation
Previously, Method/UnboundMethod basically skipped ZSUPER methods, using the super method instead. The most visible indication of this is that owner would return method referenced by the ZSUPER method, instead of the class containing the ZSUPER method. It also had the result that calling super_method on the method resulted in the super method of the method referenced by ZSUPER. Fix this by tracking the ZSUPER class, and using it as the owner. Additionally, if super_method is called on a ZSUPER method, use the existing method entry (which already points to ZSUPER's super method), instead of looking for the super method of that method entry. This has the effect of making a ZSUPER method treated more similarly to a regularly defined method that calls super. Fixes [Bug #18729]
| mod = EnvUtil.labeled_module("Mod") {private def foo; :ok end} | ||
| mods = [mod] | ||
| obj = Object.new.extend(mod) | ||
| mods = [obj.singleton_class, mod] |
There was a problem hiding this comment.
Interesting, so now the expected result is [sclass, Mod, Mod0, Mod1], which is what would be the chain if super was caller in each of these methods, so that makes a lot of sense.
There was a problem hiding this comment.
It's actually [Mod0, Mod1, sclass, Mod], since Mod0 and Mod1 are prepended to the singleton class.
There was a problem hiding this comment.
Actually, on a similar simplified example, calling these methods means only the public foo is called, and the private one is skipped:
class C
def foo
puts caller(0), nil
['C']
end
end
module Mod
private def foo
['.'] + super
end
end
obj = C.new
obj.extend(Mod)
class << obj
public :foo
end
p obj.foogives (on 3.0.3)
public.rb:3:in `foo'
public.rb:10:in `foo'
public.rb:21:in `<main>'
[".", "C"]
That's I think because super doesn't consider the owner module but instead the "declaring module" (the one that actually def it, I think that's defined_class in CRuby terminology), which for both sclass#foo and Mod#foo is Mod.
And there is a similar cases with aliases IIRC.
There was a problem hiding this comment.
So I think ideally super_method wouldn't change here, and the result of this test would be [Mod0, Mod1, sclass].
That's what actually gets called if super is used, and it's also the result of this test on TruffleRuby.
eregon
left a comment
There was a problem hiding this comment.
Looks great to me.
I'm not familiar with the module chain details on CRuby though, so if you'd like a review of that it seems best to ask someone else.
Previously, Method/UnboundMethod basically skipped ZSUPER methods,
using the super method instead. The most visible indication of
this is that owner would return method referenced by the ZSUPER
method, instead of the class containing the ZSUPER method. It
also had the result that calling super_method on the method
resulted in the super method of the method referenced by ZSUPER.
Fix this by tracking the ZSUPER class, and using it as the owner.
Additionally, if super_method is called on a ZSUPER method, use
the existing method entry (which already points to ZSUPER's super
method), instead of looking for the super method of that method
entry.
This has the effect of making a ZSUPER method treated more similarly
to a regularly defined method that calls super.
Fixes [Bug #18729]