Skip to content

Refactor keyword arguments logic#9195

Merged
headius merged 2 commits intojruby:masterfrom
headius:kwargs_refactor
Feb 3, 2026
Merged

Refactor keyword arguments logic#9195
headius merged 2 commits intojruby:masterfrom
headius:kwargs_refactor

Conversation

@headius
Copy link
Member

@headius headius commented Jan 29, 2026

This PR will refactor our keyword arguments logic to accomplish the following:

  • Simpler to follow logic
  • Unified logic for both interpreter and JIT
  • Easing the path toward on-stack keyword arguments handling

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.

@headius headius added this to the JRuby 10.0.4.0 milestone Jan 29, 2026
@headius headius force-pushed the kwargs_refactor branch 3 times, most recently from 206cd22 to 8ac492f Compare January 29, 2026 03:58
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.
@headius headius force-pushed the kwargs_refactor branch 3 times, most recently from bffb06a to 78abdc8 Compare January 29, 2026 05:15
@headius headius marked this pull request as draft January 29, 2026 06:17
Copy link
Member

@enebo enebo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@headius
Copy link
Member Author

headius commented Jan 30, 2026

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.
@headius headius marked this pull request as ready for review February 3, 2026 18:20
@headius headius merged commit 39ba94f into jruby:master Feb 3, 2026
77 checks passed
@headius headius deleted the kwargs_refactor branch February 3, 2026 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants