Fix original_name when two methods share a wrapper (like is_a? kind_of?) - #9441
Merged
Merged
Conversation
Fix Method#original_name and #inspect when two methods share a wrapper (like is_a?/kind_of?) Introduce sourceName on DynamicMethod to preserve a method's source name when methods share a single underlying wrapper. Set it in RubyModule when binding, walk the alias chain in AliasMethod#getOldName to reach it, and consult it from Method#original_name and #inspect so both report the correct source name instead of whichever name happened to live on the wrapper. Covered by new specs in ruby/spec#1356 (ruby/spec#1356).
sampokuokkanen
force-pushed
the
kernel-alises-fix
branch
from
May 12, 2026 13:36
85f652d to
b703780
Compare
headius
requested changes
May 12, 2026
headius
left a comment
headius
left a comment
Member
There was a problem hiding this comment.
I think we can distill this down a bit:
- We don't need both
getOldNameandgetSourceName. We should either lift the former up or deprecate it and use the latter, I think. - AliasMethod should override whichever method is the standard "get original method name" going forward, so we're not checking for AliasMethod and null values all over.
- The
sourceNamestored inDynamicMethodcan be final, I think. It is only initialized during method duping, which could call an internal constructor.
sampokuokkanen
force-pushed
the
kernel-alises-fix
branch
2 times, most recently
from
May 18, 2026 03:06
ce24e5a to
4ee8bcb
Compare
Replace the mutable sourceName field on DynamicMethod with RenamedDynamicMethod, a thin DelegatingDynamicMethod subclass that records the source name in a final field. Each define_method(name, Method) call now produces its own wrapper, so per-binding state is naturally separate without any mutation, and setSourceName drops off DynamicMethod's API.
sampokuokkanen
force-pushed
the
kernel-alises-fix
branch
from
May 18, 2026 03:32
4ee8bcb to
0d5e85e
Compare
Contributor
Author
|
Refactored: for |
headius
approved these changes
Sep 2, 2026
headius
left a comment
headius
left a comment
Member
There was a problem hiding this comment.
Much cleaner after updates. Sorry for the delay... approved!
sampokuokkanen
added a commit
to sampokuokkanen/jruby
that referenced
this pull request
Sep 2, 2026
PR jruby#9441 (RenamedDynamicMethod) makes Method/UnboundMethod report the source name, so these no longer fail: Method#original_name / UnboundMethod#original_name (both examples) Method#to_s / UnboundMethod#to_s "shows the source UnboundMethod's name in parentheses" and "shows the source name when aliasing" inspect_tags.txt for both classes are stale: inspect_spec.rb now only holds the "is an alias of #to_s" example, which passes. Still tagged (known limitation from jruby#9441 and the multi-name @JRubyMethod equality issue): Method#=== / #eql?, UnboundMethod#eql? "is an alias of" Method#to_s / UnboundMethod#to_s "does not annotate a directly looked-up Kernel method with a shared internal name" Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HgsE3g8bZfLw3mMPvei6Rt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preserve a method's source name when methods share a single underlying wrapper, so
Method#original_nameand#inspectreport the right name instead of whichever name happened to live on the shared wrapper. The dup'd method is wrapped in a per-bindingRenamedDynamicMethod(aDelegatingDynamicMethodsubclass) holding the source name in a final field.Method#original_nameconsultsDynamicMethod#getOldName, which the wrapper overrides andAliasMethodwalks through.Covered by new specs in ruby/spec#1356 (ruby/spec#1356).
Sample (the case this PR fixes):
Known limitation: direct lookup of multi-name
@JRubyMethodmethods still leaks the underlying name (Class.new.method(:is_a?).inspectstill shows(kind_of?)). The specs in ruby/spec#1356 for the inspect case for likeClass.new.method(:id_a?)will fail, but I think that would be better tackled in a follow-up PR.This PR fixes the first part of #9257.
@headius
Thanks for explaining to me how method lookup works in JRuby in #9420.