gh-154115: Fix typing._tp_cache calling the wrapped function twice on TypeError#154119
Open
fedonman wants to merge 3 commits into
Open
gh-154115: Fix typing._tp_cache calling the wrapped function twice on TypeError#154119fedonman wants to merge 3 commits into
fedonman wants to merge 3 commits into
Conversation
…ice on TypeError _tp_cache falls back to calling the wrapped function directly when the arguments are unhashable, which it detected by catching TypeError from the lru_cache lookup. A TypeError raised by the wrapped function itself was caught the same way, so with hashable arguments the function ran once inside the cache, was caught, and then ran a second time through the fallback before the error propagated. This was reachable through the public API, for example typing.List[int, str] evaluated its whole __getitem__ twice. Only fall back when the arguments really are unhashable, by rebuilding the same cache key lru_cache uses, and re-raise otherwise so the function is not run a second time.
Contributor
Author
|
@zware part of EuroPython sprint. |
sobolevn
reviewed
Jul 19, 2026
…riations Both tests exercised the same positional and keyword call variations but handled them inconsistently: one cleared the calls list between them, the other asserted the combined result. Loop over the variations with subTest and clear the list each iteration so each variation is asserted in isolation.
JelleZijlstra
left a comment
Member
There was a problem hiding this comment.
I'm not convinced we should change anything here. The issue description doesn't show any evidence that this causes practical problems, and the proposed code change makes things more complicated.
Contributor
Author
|
@JelleZijlstra Should I close the PR then? |
Member
|
We can wait for some other opinions. |
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.
_tp_cachewraps typing's__getitem__methods with anlru_cacheand falls back to calling the original function when it sees aTypeError, on the assumption that theTypeErrorcame from hashing an unhashable argument. ATypeErrorraised by the wrapped function itself was caught the same way, so with hashable arguments the function ran once inside the cache and then a second time through the fallback before the error propagated.You can see it directly:
and through the public API, for example
typing.List[int, str]runs its whole__getitem__twice before raising.The fix only takes the fallback when the arguments really are unhashable. It rebuilds the same cache key that
lru_cachebuilds (functools._make_key, which also folds in the argument types whentyped=True) and re-raises when that key is hashable, so aTypeErrorcoming from the wrapped function is no longer turned into a second call. The successful path is unchanged.