-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Use all reachable bindings for instance attributes and deferred lookups #18955
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
Contributor
|
ead2611 to
085b8b4
Compare
085b8b4 to
7637047
Compare
sharkdp
added a commit
that referenced
this pull request
Jun 27, 2025
## Summary Add a benchmark for the problematic case in astral-sh/ty#711, which will potentially be solved in #18955
7637047 to
e1ee4b4
Compare
CodSpeed Instrumentation Performance ReportMerging #18955 will improve performances by ×12Comparing Summary
Benchmarks breakdown
|
sharkdp
commented
Jun 27, 2025
crates/ty_python_semantic/resources/mdtest/annotations/stdlib_typing_aliases.md
Outdated
Show resolved
Hide resolved
e1ee4b4 to
e47960e
Compare
AlexWaygood
approved these changes
Jul 1, 2025
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.
Summary
Remove a hack in control flow modeling that was treating
returnstatements at the end of function bodies in a special way (basically considering the state just before thereturnstatement as the end-of-scope state). This is not needed anymore now that #18750 has been merged.In order to make this work, we now use all reachable bindings for purposes of finding implicit instance attribute assignments as well as for deferred lookups of symbols. Both would otherwise be affected by this change:
Implicit instance attributes also required another change. We previously kept track of possibly-unbound instance attributes in some cases, but we now give up on that completely and always consider implicit instance attributes to be bound if we see a reachable binding in a reachable method. The previous behavior was somewhat inconsistent anyway because we also do not consider attributes possibly-unbound in other scenarios: we do not (and can not) keep track of whether or not methods are called that define these attributes.
closes astral-sh/ty#711
Ecosystem analysis
I think this looks very positive!
possibly-unbound-attributediagnostics (599), mostly for classes that define attributes intry … exceptblocks,forloops, orif … else: raise …constructs. There might obviously also be true positives that got removed, but the vast majority should be false positives.possibly-unresolved-reference/unresolved-referencediagnostics (279+13) from the change to deferred lookups.invalid-type-formfalse positives got resolved (13), because we can now properly look up the names in the annotations.attrs, since we understand theAttributeannotation that was previously inferred asUnknownbecause of a re-assignment after the class definition.Test Plan
The existing attributes.md test suite has sufficient coverage here.