Refactor keyword arguments logic#9195
Merged
headius merged 2 commits intojruby:masterfrom Feb 3, 2026
Merged
Conversation
206cd22 to
8ac492f
Compare
This logic decides whether to: * dup an incoming keyword arguments hash * replace the original argument with the dup * set or clear ruby2_keywords_hash flag * use the original, dup, or nothing for kwargs processing The conditions leading to these various actions were intermingled with performing the actions, making it difficult to adjust that logic without breaking unrelated code. The new logic defines an enum of all combinations of behavior and provides a straightforward set of if/else branches to choose an action. This allows both the interpreter and the JIT to use the same conditional logic but with each handling the actions in an appropriate way (e.g. JIT may not have an arguments array but it can replace fixed-arity positional arguments). First stage of refactoring keyword arguments logic to unify interpreter and JIT and make it easier to follow and maintain.
bffb06a to
78abdc8
Compare
enebo
approved these changes
Jan 30, 2026
Member
enebo
left a comment
There was a problem hiding this comment.
Looks good. We could also consider this for 10.0.4.0 since I think 4 and 3.4 more or less are the same here. I would not want it in 10.0.3.0 since I think we want some bake on this.
Member
Author
|
This is still based on master so 10.0.4.0 seems like a good target. If we can clean up/simplify/fix this logic further, all the better. At least it's easier to follow now. |
This follows the refactor of variable-arity varargs logic, shared by both JIT and interpreter. With specific arity, we never have an action that replaces the incoming argument, nor do we return UNDEFINED or clear the ruby2_keywords_hash flag.
78abdc8 to
8146a9b
Compare
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
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.
This PR will refactor our keyword arguments logic to accomplish the following:
This is in response to yet another small keyword arguments bug #8976 brought about by making a small change in JIT logic that did not match the completely different-looking interpreter logic. Having a single, unified, readable path will make many things easier.
This is originally targeted at 10.0 but the risks of merging a large refactor may mean it goes into 10.1.