-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy #17451
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
Conversation
|
|
I'm going to go ahead and land this sans review, to get the property tests green again. Post-land review welcome! |
dcreager
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
|
Huh, why didn't that show up on the PR CI? Did we recently change our default Python version on main? Thanks for fixing! |
|
Just a merge race with 9c47b6d, I think! |
|
Oh right, of course! |
* main: (123 commits) [red-knot] Handle explicit class specialization in type expressions (#17434) [red-knot] allow assignment expression in call compare narrowing (#17461) [red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy (#17451) [red-knot] Type narrowing for assertions (take 2) (#17345) [red-knot] class bases are not affected by __future__.annotations (#17456) [red-knot] Add support for overloaded functions (#17366) [`pyupgrade`] Add fix safety section to docs (`UP036`) (#17444) [red-knot] more type-narrowing in match statements (#17302) [red-knot] Add some narrowing for assignment expressions (#17448) [red-knot] Understand `typing.Protocol` and `typing_extensions.Protocol` as equivalent (#17446) Server: Use `min` instead of `max` to limit the number of threads (#17421) [red-knot] Detect version-related syntax errors (#16379) [`pyflakes`] Add fix safety section (`F841`) (#17410) [red-knot] Add `KnownFunction` variants for `is_protocol`, `get_protocol_members` and `runtime_checkable` (#17450) Bump 0.11.6 (#17449) Auto generate `visit_source_order` (#17180) [red-knot] Initial tests for protocols (#17436) [red-knot] Dataclasses: synthesize `__init__` with proper signature (#17428) [red-knot] Dataclasses: support `order=True` (#17406) [red-knot] Super-basic generic inference at call sites (#17301) ...
|
@carljm I'm not sure this PR fixed things. #17478 seems to be this same issue repeating itself :(
Huh -- if I checkout this commit, the property tests still fail for me locally. 100,000 might not be enough iterations to reliably check whether the tests are likely to be flaky -- I usually run them with |
## Summary #17451 was incomplete. `AlwaysFalsy` and `AlwaysTruthy` are not the only two types that are super-types of some literals (of a given kind) and not others. That set also includes intersections containing `AlwaysTruthy` or `AlwaysFalsy`, and intersections containing literal types of the same kind. Cover these cases as well. Fixes #17478. ## Test Plan Added mdtests. `QUICKCHECK_TESTS=1000000 cargo test -p red_knot_python_semantic -- --ignored types::property_tests::stable` failed on both `all_fully_static_type_pairs_are_subtypes_of_their_union` and `all_type_pairs_are_assignable_to_their_union` prior to this PR, passes after it. --------- Co-authored-by: Alex Waygood <[email protected]>
In #17403 I added a comment asserting that all same-kind literal types share all the same super-types. This is true, with two notable exceptions: the types
AlwaysTruthyandAlwaysFalsy. These two types are super-types of some literal types within a given kind and not others:Literal[0],Literal[""], andLiteral[b""]inhabitAlwaysFalsy, while other literals inhabitAlwaysTruthy.This PR updates the literal-unions optimization to handle these types correctly.
Fixes #17447
Verified locally that
QUICKCHECK_TESTS=100000 cargo test -p red_knot_python_semantic -- --ignored types::property_tests::stablenow passes again.