Don't copy the singleton class in Proc#dup#9523
Merged
Merged
Conversation
kares
reviewed
Jul 12, 2026
| private RubyProc procDup() { | ||
| return newProc(getRuntime(), getMetaClass(), block, type, file, line); | ||
| // The real class, not the receiver's metaclass: dup must not carry the receiver's | ||
| // singleton class over to the copy (MRI drops it), and dupSetup does not reset the |
Member
There was a problem hiding this comment.
this is fairly apparent from the code itself, no need for LLM generated 5 line comment (also okay if you just compress and keep these 2 lines).
| # Proc#dup went through the receiver's metaclass, so a proc with singleton | ||
| # methods handed its singleton state to the dup (MRI drops it; only clone | ||
| # keeps a copy of the singleton class). | ||
| describe 'Proc#dup' do |
Member
There was a problem hiding this comment.
the regression spec suite is a bit deprecated, these would make sense as ruby/spec (there might be one already, maybe it's just excluded)
Member
|
👍 good catch |
procDup() passed getMetaClass() to the copy, which is the receiver's
singleton class when the proc has singleton methods, so the dup
unexpectedly responded to the receiver's singleton methods; MRI drops
the singleton class on dup:
pr = proc { }
def pr.tag; end
pr.dup.respond_to?(:tag) # JRuby: true, MRI: false
Pass the real class instead. Proc#clone is unaffected because
cloneSetup already replaces the copy's metaclass with a clone of the
receiver's singleton class, and Proc subclasses are preserved since
the real class of a subclass instance is the subclass itself.
Co-Authored-By: Claude Fable 5 <[email protected]>
shugo
force-pushed
the
fix-proc-dup-singleton
branch
from
July 12, 2026 09:28
6e18256 to
a00577a
Compare
Contributor
Author
|
@kares Thanks for your review. I've removed obvious comments, and have removed the regression spec. |
headius
approved these changes
Jul 21, 2026
Member
|
Thanks @shugo! |
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
3 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.
procDup() passed getMetaClass() to the copy, which is the receiver's singleton class when the proc has singleton methods, so the dup unexpectedly responded to the receiver's singleton methods; MRI drops the singleton class on dup:
Pass the real class instead. Proc#clone is unaffected because cloneSetup already replaces the copy's metaclass with a clone of the receiver's singleton class, and Proc subclasses are preserved since the real class of a subclass instance is the subclass itself.