fix: handle VAR_POSITIONAL and kwargs precedence in _fast_bind cache#556
Merged
fix: handle VAR_POSITIONAL and kwargs precedence in _fast_bind cache#556
Conversation
The _fast_bind method had three bugs when replaying cached binding templates: 1. VAR_POSITIONAL (*args) params were stored as a single value (args[i]) instead of a tuple (args[i:]), causing TypeError when BoundArguments.args tried to extend() a non-iterable value. 2. KEYWORD_ONLY params after *args were not processed at all — the loop would break at the VAR_POSITIONAL and skip remaining params. 3. POSITIONAL_OR_KEYWORD params that share a name with a kwarg key should prefer the kwargs value (matching _full_bind behavior), but _fast_bind always used the positional arg. These bugs only manifested when the bind cache was shared across tests via the module-level signature_cache (different callables with the same signature sharing a SignatureAdapter instance). Fixes issues observed in SCXML W3C tests (test179, test527, test529, test562, test578) when run together with the bind cache from #550.
|
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
1 participant
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.
Summary
Fixes three bugs in the
_fast_bindcached path introduced by #550:*args): Was stored as rawargs[i]instead ofargs[i:]tuple, causingTypeError: 'int' object is not iterableinBoundArguments.args*args: Params after*args(e.g.,def fn(*args, event, **kwargs)) were skipped entirely — the loop broke at VAR_POSITIONAL without processing remaining keyword-only params_full_bindprefers kwargs, but_fast_bindalways used the positional argThese bugs only manifest when the
_bind_cache(on a sharedSignatureAdapterinstance) is populated by one call shape and reused by another — e.g., across SCXML test cases that share callbacks viasignature_cache.Test plan
test_var_positional_collected_as_tuple— verifies*argscollected as tuple on cache hittest_keyword_only_after_var_positional— verifies KEYWORD_ONLY params after*argsworktest_positional_or_keyword_prefers_kwargs_over_positional— verifies kwargs precedencetest_empty_var_positional— verifies empty*argsedge casetest_named_params_before_var_positional— verifies named params before*argstest_kwargs_wins_with_var_positional_present— verifies kwargs precedence with*argsFixes SCXML W3C test failures (test179, test527, test529, test562, test578) on the
macedo/scxmlbranch CI.