Skip to content

Comments

[red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy#17451

Merged
carljm merged 1 commit intomainfrom
cjm/unionfix
Apr 18, 2025
Merged

[red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy#17451
carljm merged 1 commit intomainfrom
cjm/unionfix

Conversation

@carljm
Copy link
Contributor

@carljm carljm commented Apr 17, 2025 •
edited
Loading

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 AlwaysTruthy and AlwaysFalsy. These two types are super-types of some literal types within a given kind and not others: Literal[0], Literal[""], and Literal[b""] inhabit AlwaysFalsy, while other literals inhabit AlwaysTruthy.

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::stable now passes again.

@carljm carljm added the ty Multi-file analysis & type inference label Apr 17, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 17, 2025

mypy_primer results

No ecosystem changes detected ✅

@carljm
Copy link
Contributor Author

carljm commented Apr 18, 2025

I'm going to go ahead and land this sans review, to get the property tests green again. Post-land review welcome!

@carljm carljm merged commit 84d064a into main Apr 18, 2025
23 checks passed
@carljm carljm deleted the cjm/unionfix branch April 18, 2025 15:20
@carljm carljm mentioned this pull request Apr 18, 2025
Closed
dcreager
dcreager reviewed Apr 18, 2025
View reviewed changes
Copy link
Member

@dcreager dcreager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

dcreager added a commit that referenced this pull request Apr 18, 2025
@dcreager
Merge branch 'main' into dcreager/subscript-types
967c8ea 
* main:
  [red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy (#17451)
  [red-knot] Type narrowing for assertions (take 2) (#17345)
@dcreager
Copy link
Member

dcreager commented Apr 18, 2025 •
edited
Loading

This added a test regression on main, since the new mdtests use a type statement, which isn't available until 3.12. I've got a fix (0b116e5) ready to go on #17434, which I am about to merge once CI (hopefully) goes green.

@carljm
Copy link
Contributor Author

carljm commented Apr 18, 2025

Huh, why didn't that show up on the PR CI? Did we recently change our default Python version on main? Thanks for fixing!

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 18, 2025

Just a merge race with 9c47b6d, I think!

@carljm
Copy link
Contributor Author

carljm commented Apr 18, 2025

Oh right, of course!

@dcreager
Copy link
Member

dcreager commented Apr 18, 2025

Huh, why didn't that show up on the PR CI? Did we recently change our default Python version on main? Thanks for fixing!

@ntBre added version-related syntax errors after the point on main where you had branched from: #16379

dcreager added a commit that referenced this pull request Apr 18, 2025
@dcreager
Merge branch 'main' into dcreager/legacy-typevar-instance
fd91822 
* 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)
  ...
@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 19, 2025 •
edited
Loading

@carljm I'm not sure this PR fixed things. #17478 seems to be this same issue repeating itself :(

Verified locally that QUICKCHECK_TESTS=100000 cargo test -p red_knot_python_semantic -- --ignored types::property_tests::stable now passes again.

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 QUICKCHECK_TESTS=1000000 these days:

HEAD is now at 84d064a14 [red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy (#17451)
~/dev/ruff (84d064a14)⚡ % QUICKCHECK_TESTS=1000000 cargo test --release -p red_knot_python_semantic -- --ignored types::property_tests::stable
   Compiling ruff_python_ast v0.0.0 (/Users/alexw/dev/ruff/crates/ruff_python_ast)
   Compiling red_knot_python_semantic v0.0.0 (/Users/alexw/dev/ruff/crates/red_knot_python_semantic)
   Compiling ruff_python_parser v0.0.0 (/Users/alexw/dev/ruff/crates/ruff_python_parser)
   Compiling ruff_python_literal v0.0.0 (/Users/alexw/dev/ruff/crates/ruff_python_literal)
   Compiling ruff_db v0.0.0 (/Users/alexw/dev/ruff/crates/ruff_db)
   Compiling red_knot_vendored v0.0.0 (/Users/alexw/dev/ruff/crates/red_knot_vendored)
   Compiling red_knot_test v0.0.0 (/Users/alexw/dev/ruff/crates/red_knot_test)
    Finished `release` profile [optimized] target(s) in 24.64s
     Running unittests src/lib.rs (target/release/deps/red_knot_python_semantic-bb028dec96527f17)

running 23 tests
test types::property_tests::stable::all_fully_static_type_pairs_are_subtype_of_their_union ... FAILED
test types::property_tests::stable::all_type_pairs_are_assignable_to_their_union ... FAILED
test types::property_tests::stable::all_fully_static_types_subtype_of_object has been running for over 60 seconds
test types::property_tests::stable::all_types_assignable_to_object has been running for over 60 seconds
test types::property_tests::stable::assignable_to_is_reflexive has been running for over 60 seconds
test types::property_tests::stable::disjoint_from_is_irreflexive has been running for over 60 seconds
test types::property_tests::stable::disjoint_from_is_symmetric has been running for over 60 seconds
test types::property_tests::stable::equivalent_to_is_reflexive has been running for over 60 seconds
test types::property_tests::stable::equivalent_to_is_symmetric has been running for over 60 seconds
test types::property_tests::stable::equivalent_to_is_transitive has been running for over 60 seconds
test types::property_tests::stable::gradual_equivalent_to_is_symmetric has been running for over 60 seconds
test types::property_tests::stable::never_assignable_to_every_type has been running for over 60 seconds
test types::property_tests::stable::never_subtype_of_every_fully_static_type has been running for over 60 seconds
test types::property_tests::stable::non_fully_static_types_do_not_participate_in_equivalence has been running for over 60 seconds
test types::property_tests::stable::never_assignable_to_every_type ... ok
test types::property_tests::stable::equivalent_to_is_reflexive ... ok
test types::property_tests::stable::never_subtype_of_every_fully_static_type ... ok
test types::property_tests::stable::assignable_to_is_reflexive ... ok
test types::property_tests::stable::non_fully_static_types_do_not_participate_in_subtyping has been running for over 60 seconds
test types::property_tests::stable::disjoint_from_is_irreflexive ... ok
test types::property_tests::stable::all_fully_static_types_subtype_of_object ... ok
test types::property_tests::stable::all_types_assignable_to_object ... ok
test types::property_tests::stable::singleton_implies_single_valued has been running for over 60 seconds
test types::property_tests::stable::singleton_implies_single_valued ... ok
test types::property_tests::stable::non_fully_static_types_do_not_participate_in_equivalence ... ok
test types::property_tests::stable::gradual_equivalent_to_is_symmetric ... ok
test types::property_tests::stable::equivalent_to_is_symmetric ... ok
test types::property_tests::stable::subtype_of_is_reflexive ... ok
test types::property_tests::stable::subtype_of_implies_assignable_to has been running for over 60 seconds
test types::property_tests::stable::subtype_of_implies_not_disjoint_from has been running for over 60 seconds
test types::property_tests::stable::subtype_of_is_antisymmetric has been running for over 60 seconds
test types::property_tests::stable::non_fully_static_types_do_not_participate_in_subtyping ... ok
test types::property_tests::stable::subtype_of_is_transitive has been running for over 60 seconds
test types::property_tests::stable::disjoint_from_is_symmetric ... ok
test types::property_tests::stable::two_equivalent_types_are_also_gradual_equivalent has been running for over 60 seconds
test types::property_tests::stable::two_gradual_equivalent_fully_static_types_are_also_equivalent has been running for over 60 seconds
test types::property_tests::stable::equivalent_to_is_transitive ... ok
test types::property_tests::stable::subtype_of_implies_assignable_to ... ok
test types::property_tests::stable::subtype_of_is_antisymmetric ... ok
test types::property_tests::stable::subtype_of_implies_not_disjoint_from ... ok
test types::property_tests::stable::two_equivalent_types_are_also_gradual_equivalent ... ok
test types::property_tests::stable::two_gradual_equivalent_fully_static_types_are_also_equivalent ... ok
test types::property_tests::stable::subtype_of_is_transitive ... ok

failures:

---- types::property_tests::stable::all_fully_static_type_pairs_are_subtype_of_their_union stdout ----

thread 'types::property_tests::stable::all_fully_static_type_pairs_are_subtype_of_their_union' panicked at /Users/alexw/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/quickcheck-1.0.3/src/tester.rs:165:28:
[quickcheck] TEST FAILED. Arguments: (Union([BytesLiteral("\0"), BytesLiteral("")]), Intersection { pos: [], neg: [BytesLiteral("")] })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- types::property_tests::stable::all_type_pairs_are_assignable_to_their_union stdout ----

thread 'types::property_tests::stable::all_type_pairs_are_assignable_to_their_union' panicked at /Users/alexw/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/quickcheck-1.0.3/src/tester.rs:165:28:
[quickcheck] TEST FAILED. Arguments: (Union([BytesLiteral("\0"), BytesLiteral("")]), Intersection { pos: [], neg: [AlwaysFalsy] })


failures:
    types::property_tests::stable::all_fully_static_type_pairs_are_subtype_of_their_union
    types::property_tests::stable::all_type_pairs_are_assignable_to_their_union

test result: FAILED. 21 passed; 2 failed; 0 ignored; 0 measured; 210 filtered out; finished in 162.30s

error: test failed, to rerun pass `-p red_knot_python_semantic --lib`

@AlexWaygood AlexWaygood mentioned this pull request Apr 20, 2025
Closed
@carljm carljm mentioned this pull request Apr 22, 2025
Merged
carljm added a commit that referenced this pull request Apr 22, 2025
@carljm @AlexWaygood
[red-knot] fix unions of literals, again (#17534)
27ada26 
## 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dcreager dcreager dcreager left review comments

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood

@sharkdp sharkdp Awaiting requested review from sharkdp

Assignees

No one assigned

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[red-knot] Some pairs of types are no longer recognised as assignable to their union

3 participants

@carljm @dcreager @AlexWaygood