[ty] Improve sys.version_info special casing#19894
Conversation
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-08-13 13:09:38.590966201 +0000
+++ new-output.txt 2025-08-13 13:09:38.658966248 +0000
@@ -1,5 +1,5 @@
WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(34b8)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(154c3)): execute: too many cycle iterations`
_directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
_directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
_directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__` |
|
We need @carljm to fix the remaining recursive type / generic issues. The conformance test panic is irritating ;) (that's supposed to be pep talk, but I know, I'm not very good at it) |
|
| } | ||
|
|
||
| /// Return the `TupleSpec` for the singleton `sys.version_info` | ||
| pub(crate) fn version_info_spec(db: &'db dyn Db) -> TupleSpec<'db> { |
There was a problem hiding this comment.
It's fairly easy to add caching to this method if we revert d76fd10, but I still haven't been able to find any evidence yet that this would provide a significant speedup. I also haven't tried doing that as an isolated change on the codspeed runners, though; I've only experimented by running benchmarks locally.
| let tuple_type = TupleType::new(db, TupleSpec::version_info_spec(db)) | ||
| .expect("sys.version_info tuple spec should always be a valid tuple"); |
There was a problem hiding this comment.
Just something to be aware of. This has a risk of lock congestion if explict_bases is very hot because we keep interning the same TupleType. I don't think explict_bases is that hot that this is an issue here but I thought it's worth noting (and it's also something that we might just need to fix in Salsa)
There was a problem hiding this comment.
I think this is likely not an issue here because we only intern when we call explicit_bases on the VersionInfo class specifically. And this method is cached so if we do that frequently, all but the first time in a revision wouldn't reach here anyway.
* main: [ty] Add precise inference for indexing, slicing and unpacking `NamedTuple` instances (#19560) Add rule code to GitLab description (#19896) [ty] render docstrings in hover (#19882) [ty] simplify return type of place_from_declarations (#19884) [ty] Various minor cleanups to tuple internals (#19891) [ty] Improve `sys.version_info` special casing (#19894)
…aints * dcreager/inferrable: (65 commits) this was right after all mark typevars inferrable as we go fix tests fix inference of constrained typevars [ty] Add precise inference for indexing, slicing and unpacking `NamedTuple` instances (#19560) Add rule code to GitLab description (#19896) [ty] render docstrings in hover (#19882) [ty] simplify return type of place_from_declarations (#19884) [ty] Various minor cleanups to tuple internals (#19891) [ty] Improve `sys.version_info` special casing (#19894) Don't cache files with diagnostics (#19869) [ty] support recursive type aliases (#19805) [ty] Remove unsafe `salsa::Update` implementations in `tuple.rs` (#19880) [ty] Function argument inlay hints (#19269) [ty] Remove Salsa interning for `TypedDictType` (#19879) Fix `lint.future-annotations` link (#19876) [ty] Reduce memory usage of `TupleSpec` and `TupleType` (#19872) [ty] Track heap usage of salsa structs (#19790) Update salsa to pull in tracked struct changes (#19843) [ty] simplify CycleDetector::visit signature (#19873) ...
3 participants
Summary
Currently the
__getitem__method we infer forsys.version_infois inconsistent with the types we infer for indexing/slicing/unpackingsys.version_info. This isn't really a big deal, but... it would be nice to be consistent :-)The reason for the inconsistency is that we infer
sys.version_infoas inheriting fromtuple[int, int, int, @Todo(Type Alias), int]due to typeshed's definition, but our special casing elsewhere treats it as a subtype oftuple[Literal[3], Literal[<minor Python version>], int, Literal["alpha", "beta", ""candidate", "final"], int]. We can resolve the inconsistency by adding some special-casing forsys.version_infotoClassLiteral::explicit_bases().In theory adding the special casing to
ClassLiteral::explicit_bases()would let us remove any special casing forsys.version_infofrominstance.rs: we could just iterate through the MRO ofsys._version_infountil we find the tuple type in its MRO, and return the tuple spec of that tuple type. In practice, that (unsurpringly) leads to a big performance regression, however.I'm adding the "internal" label since I doubt this really changes any behaviour that users would realistically care about.
Test Plan
Mdtests added