Bug #22022
closedRefinement zsuper problems when referenced method changes
Description
I've found many problems with refinement zsuper methods. Here's one example:
module M
private def a = :a
alias a a
end
class A
include M
end
class B < A
end
module R
refine B do
public :a
end
end
using R
B.new.a
# false() function is unimplemented on this machine (NotImplementedError)
Here's another:
class A
private def a = :a
end
module R
refine A do
public :a
end
end
using R
A.new.a
# => :a
module M
def a = :b
end
A.prepend M
A.new.a
# SystemStackError: stack level too deep
The problematic cases are in general can be broken down into:
- Method being redefined (in same or closer ancestor)
- Method being removed
- Method being undef-ed (in same or closer ancestor)
- Method being overridden by method in included module
- Method being overridden by method in prepended module
- Method being defined in an included/prepended module at time of refinement
In addition to the NotImplementedError and SystemStackError, there are other cases where incorrect results are given.
One possible approach for fixing this is to turn the zsuper methods into regular methods that call super . This fixes all cases that I found. This is not a perfect solution, due to mildly worse performance (I'm guessing) as well as arity/parameters for the method not as helpful.
It may possible to avoid these issues by clearing method caches in more cases. However, I think that would require a lot of extra work, since you cannot just clear the method cache for the current class. You would need to clear it for all subclasses, if this is a module, do the same for all classes that include/prepend the module, as well as any subclasses of those classes.
Considering the need for refinement zsuper methods is very rare, the performance and arity/parameters issues seem acceptable (to me).
Pull request: https://github.com/ruby/ruby/pull/16844
Updated by jeremyevans (Jeremy Evans) 19 days ago
- Status changed from Open to Closed
Applied in changeset git|fafb55877aaf34592278eb3ef9ba3f61473d0a56.
Avoid issues with refinement zsuper lookup
Without this patch, there are multiple issues when defining a zsuper
method via a refinement. These issues are due to the orig_me entry
for the zsuper method not being updated when the method it refers
to is modified via:
- Method being redefined (in same or closer ancestor)
- Method being removed
- Method being undef-ed (in same or closer ancestor)
- Method being overridden by method in included module
- Method being overridden by method in prepended module
- Method being defined in an included/prepended module at time of
refinement
This includes a comprehensive test suite for these cases, using both
zsuper methods and iseq methods that call super. Without the changes
to refinement method lookup, the following types of errors occur for
refinement zsuper methods (all of the iseq methods that call super
work correctly with or without these changes):
- Incorrect result (not returning result of expected method, not
raising NoMethodError if the method was removed or has been undef-ed) - SystemStackError: stack level too deep
- NotImplementedError: false() function is unimplemented on this machine
This avoids the issues with refinement zsuper lookup by turning the
zsuper into a cfunc that uses rb_call_super_kw. This is not a perfect
solution, for two reasons:
- cfuncs are slower than zsuper
- arity/parameters for the method not as helpful (you
It may possible to avoid these issues by clearing method caches in
more cases. However, I think that would require a lot of extra work,
since you cannot just clear the method cache for the current class.
You would need to clear it for all subclasses that are refined, and
if this is a module, do the same for all classes that include/prepend
the module, as well as any subclasses of those classes.
Considering the need for refinement zsuper methods is very rare, the
performance and arity/parameters issues seem acceptable (to me).
Fixes [Bug #22022]