-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Exhaustiveness: reveal opaque types properly #116821
Conversation
2ea5001
to
39105be
Compare
Nominating for T-lang discussion, as this allows more code to compile (both on stable, but especially on nightly around type-alias-impl-trait). See the main post for an explanation of this change along with a code example that now compiles, but didn't before |
I added a remark to the OP regarding the fact that |
At the suggestion of @oli-obk I now reveal opaque types in depth, which solves an inconsistency I had. I've updated the OP. |
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #117564) made this pull request unmergeable. Please resolve the merge conflicts. |
e822651
to
1d04764
Compare
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.
Implementation looks fine to me. Marking as waiting-on-fcp.
…-errors Exhaustiveness: reveal opaque types properly Previously, exhaustiveness had no clear policy around opaque types. In this PR I propose the following policy: within the body of an item that defines the hidden type of some opaque type, exhaustiveness checking on a value of that opaque type is performed using the concrete hidden type inferred in this body. I'm not sure how consistent this is with other operations allowed on opaque types; I believe this will require FCP. From what I can tell, this doesn't change anything for non-empty types. The observable changes are: - when the real type is uninhabited, matches within the defining scopes can now rely on that for exhaustiveness, e.g.: ```rust #[derive(Copy, Clone)] enum Void {} fn return_never_rpit(x: Void) -> impl Copy { if false { match return_never_rpit(x) {} } x } ``` - this properly fixes ICEs like rust-lang#117100 that occurred because a same match could have some patterns where the type is revealed and some where it is not. Bonus subtle point: if `x` is opaque, a match like `match x { ("", "") => {} ... }` will constrain its type ([playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=901d715330eac40339b4016ac566d6c3)). This is not the case for `match x {}`: this will not constain the type, and will only compile if something else constrains the type to be empty. Fixes rust-lang#117100 r? `@oli-obk` Edited for precision of the wording [Included](rust-lang#116821 (comment)) in the FCP on this PR is this rule: > Within the body of an item that defines the hidden type of some opaque type, exhaustiveness checking on a value of that opaque type is performed using the concrete hidden type inferred in this body.
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
The error does not seem related to this PR (and I can't reproduce it) @bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (c1fc1d1): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 673.496s -> 674.428s (0.14%) |
…mpiler-errors Exhaustiveness: Reveal empty opaques in depth Follow-up to rust-lang#116821. As noted [there](rust-lang#116821 (comment)), the current implementation doesn't detect emptiness of opaques when the opaque is nested inside a type. This doesn't matter for stable behavior (which ignores nested empty types anyway) but does matter for the [`exhaustive_patterns`](rust-lang#51085) features. This PR fixes this behavior by adding `InhabitedPredicate::apply_reveal_opaque` that considers opaque types when determining inhabitedness. r? `@compiler-errors`
Exhaustiveness: reveal opaque types properly Previously, exhaustiveness had no clear policy around opaque types. In this PR I propose the following policy: within the body of an item that defines the hidden type of some opaque type, exhaustiveness checking on a value of that opaque type is performed using the concrete hidden type inferred in this body. I'm not sure how consistent this is with other operations allowed on opaque types; I believe this will require FCP. From what I can tell, this doesn't change anything for non-empty types. The observable changes are: - when the real type is uninhabited, matches within the defining scopes can now rely on that for exhaustiveness, e.g.: ```rust #[derive(Copy, Clone)] enum Void {} fn return_never_rpit(x: Void) -> impl Copy { if false { match return_never_rpit(x) {} } x } ``` - this properly fixes ICEs like rust-lang/rust#117100 that occurred because a same match could have some patterns where the type is revealed and some where it is not. Bonus subtle point: if `x` is opaque, a match like `match x { ("", "") => {} ... }` will constrain its type ([playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=901d715330eac40339b4016ac566d6c3)). This is not the case for `match x {}`: this will not constain the type, and will only compile if something else constrains the type to be empty. Fixes rust-lang/rust#117100 r? `@oli-obk` Edited for precision of the wording [Included](rust-lang/rust#116821 (comment)) in the FCP on this PR is this rule: > Within the body of an item that defines the hidden type of some opaque type, exhaustiveness checking on a value of that opaque type is performed using the concrete hidden type inferred in this body.
Exhaustiveness: Statically enforce revealing of opaques In rust-lang#116821 it was decided that exhaustiveness should operate on the hidden type of an opaque type when relevant. This PR makes sure we consistently reveal opaques within exhaustiveness. This makes it possible to remove `reveal_opaque_ty` from the `TypeCx` trait which was an unfortunate implementation detail. r? `@compiler-errors`
…mpiler-errors Exhaustiveness: Statically enforce revealing of opaques In rust-lang#116821 it was decided that exhaustiveness should operate on the hidden type of an opaque type when relevant. This PR makes sure we consistently reveal opaques within exhaustiveness. This makes it possible to remove `reveal_opaque_ty` from the `TypeCx` trait which was an unfortunate implementation detail. r? `@compiler-errors`
…mpiler-errors Exhaustiveness: Statically enforce revealing of opaques In rust-lang#116821 it was decided that exhaustiveness should operate on the hidden type of an opaque type when relevant. This PR makes sure we consistently reveal opaques within exhaustiveness. This makes it possible to remove `reveal_opaque_ty` from the `TypeCx` trait which was an unfortunate implementation detail. r? `@compiler-errors`
…rors Exhaustiveness: Statically enforce revealing of opaques In rust-lang/rust#116821 it was decided that exhaustiveness should operate on the hidden type of an opaque type when relevant. This PR makes sure we consistently reveal opaques within exhaustiveness. This makes it possible to remove `reveal_opaque_ty` from the `TypeCx` trait which was an unfortunate implementation detail. r? `@compiler-errors`
Pkgsrc changes: * Adapt checksums and patches. Upstream chnages: Version 1.77.0 (2024-03-21) ========================== - [Reveal opaque types within the defining body for exhaustiveness checking.] (rust-lang/rust#116821) - [Stabilize C-string literals.] (rust-lang/rust#117472) - [Stabilize THIR unsafeck.] (rust-lang/rust#117673) - [Add lint `static_mut_refs` to warn on references to mutable statics.] (rust-lang/rust#117556) - [Support async recursive calls (as long as they have indirection).] (rust-lang/rust#117703) - [Undeprecate lint `unstable_features` and make use of it in the compiler.] (rust-lang/rust#118639) - [Make inductive cycles in coherence ambiguous always.] (rust-lang/rust#118649) - [Get rid of type-driven traversal in const-eval interning] (rust-lang/rust#119044), only as a [future compatiblity lint] (rust-lang/rust#122204) for now. - [Deny braced macro invocations in let-else.] (rust-lang/rust#119062) Compiler -------- - [Include lint `soft_unstable` in future breakage reports.] (rust-lang/rust#116274) - [Make `i128` and `u128` 16-byte aligned on x86-based targets.] (rust-lang/rust#116672) - [Use `--verbose` in diagnostic output.] (rust-lang/rust#119129) - [Improve spacing between printed tokens.] (rust-lang/rust#120227) - [Merge the `unused_tuple_struct_fields` lint into `dead_code`.] (rust-lang/rust#118297) - [Error on incorrect implied bounds in well-formedness check] (rust-lang/rust#118553), with a temporary exception for Bevy. - [Fix coverage instrumentation/reports for non-ASCII source code.] (rust-lang/rust#119033) - [Fix `fn`/`const` items implied bounds and well-formedness check.] (rust-lang/rust#120019) - [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.] (rust-lang/rust#118704) - Add several new tier 3 targets: - [`aarch64-unknown-illumos`] (rust-lang/rust#112936) - [`hexagon-unknown-none-elf`] (rust-lang/rust#117601) - [`riscv32imafc-esp-espidf`] (rust-lang/rust#119738) - [`riscv32im-risc0-zkvm-elf`] (rust-lang/rust#117958) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Implement `From<&[T; N]>` for `Cow<[T]>`.] (rust-lang/rust#113489) - [Remove special-case handling of `vec.split_off (0)`.](rust-lang/rust#119917) Stabilized APIs --------------- - [`array::each_ref`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref) - [`array::each_mut`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut) - [`core::net`] (https://doc.rust-lang.org/stable/core/net/index.html) - [`f32::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even) - [`f64::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even) - [`mem::offset_of!`] (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html) - [`slice::first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk) - [`slice::first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut) - [`slice::split_first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk) - [`slice::split_first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut) - [`slice::last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk) - [`slice::last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut) - [`slice::split_last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk) - [`slice::split_last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut) - [`slice::chunk_by`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by) - [`slice::chunk_by_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut) - [`Bound::map`] (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map) - [`File::create_new`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new) - [`Mutex::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison) - [`RwLock::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison) Cargo ----- - [Extend the build directive syntax with `cargo::`.] (rust-lang/cargo#12201) - [Stabilize metadata `id` format as `PackageIDSpec`.] (rust-lang/cargo#12914) - [Pull out as `cargo-util-schemas` as a crate.] (rust-lang/cargo#13178) - [Strip all debuginfo when debuginfo is not requested.] (rust-lang/cargo#13257) - [Inherit jobserver from env for all kinds of runners.] (rust-lang/cargo#12776) - [Deprecate rustc plugin support in cargo.] (rust-lang/cargo#13248) Rustdoc ----- - [Allows links in markdown headings.] (rust-lang/rust#117662) - [Search for tuples and unit by type with `()`.] (rust-lang/rust#118194) - [Clean up the source sidebar's hide button.] (rust-lang/rust#119066) - [Prevent JS injection from `localStorage`.] (rust-lang/rust#120250) Misc ---- - [Recommend version-sorting for all sorting in style guide.] (rust-lang/rust#115046) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Add more weirdness to `weird-exprs.rs`.] (rust-lang/rust#119028)
…rors Exhaustiveness: Statically enforce revealing of opaques In rust-lang/rust#116821 it was decided that exhaustiveness should operate on the hidden type of an opaque type when relevant. This PR makes sure we consistently reveal opaques within exhaustiveness. This makes it possible to remove `reveal_opaque_ty` from the `TypeCx` trait which was an unfortunate implementation detail. r? `@compiler-errors`
…rors Exhaustiveness: Statically enforce revealing of opaques In rust-lang/rust#116821 it was decided that exhaustiveness should operate on the hidden type of an opaque type when relevant. This PR makes sure we consistently reveal opaques within exhaustiveness. This makes it possible to remove `reveal_opaque_ty` from the `TypeCx` trait which was an unfortunate implementation detail. r? `@compiler-errors`
Previously, exhaustiveness had no clear policy around opaque types. In this PR I propose the following policy: within the body of an item that defines the hidden type of some opaque type, exhaustiveness checking on a value of that opaque type is performed using the concrete hidden type inferred in this body.
I'm not sure how consistent this is with other operations allowed on opaque types; I believe this will require FCP.
From what I can tell, this doesn't change anything for non-empty types.
The observable changes are:
Unexpected type for 'Single' constructor
#117100 that occurred because a same match could have some patterns where the type is revealed and some where it is not.Bonus subtle point: if
x
is opaque, a match likematch x { ("", "") => {} ... }
will constrain its type (playground). This is not the case formatch x {}
: this will not constain the type, and will only compile if something else constrains the type to be empty.Fixes #117100
r? @oli-obk
Edited for precision of the wording
Included in the FCP on this PR is this rule: