So the recent change in PR 38069 to make the exhaustiveness checking consider uninhabited types specially caused a lot fallout. A lot of this is from crates that use #[deny(warnings)]
, and hence which are now failing with arms that used to be considered inhabited being considered uninhabited, but some of it is not. Lint failures are not in and of themselves worrisome, but I am a bit worried that we have changed the semantics of code in unacceptable ways. Here is a list of the issues opened tallying regressions that I am aware of:
-
https://github.com/rust-lang/rust/issues/38889 – Subtle breaking change with match patterns on uninhabited types
- Points out that empty enums were being used intentionally to make “placeholder” enum variants, but these placeholders are now recognized as impossible.
- There exists, I think, a better pattern that could be used here.
-
https://github.com/rust-lang/rust/issues/38969 – ICE in empty-0.0.4, Rust 1.16, unreachable for-loop pattern
- A problem because it is a hard error for a
for loop
pattern to be unreachable. This is fixable and should not be a hard error anyway, in my opinion.
- A problem because it is a hard error for a
-
https://github.com/rust-lang/rust/issues/38972 – Irrefutable while-let pattern in log4rs-rolling-file-0.2.0, Rust 1.16
- A problem because it is a hard error for a
while let
pattern to be irrefutable. This is fixable and should not be a hard error anyway, in my opinion.
- A problem because it is a hard error for a
-
https://github.com/rust-lang/rust/issues/38977 – Unreachable expression in guard-0.3.1, Rust 1.15
- A problem only because the code is used in macros that gets exported to clients, which may have
#[deny(warnings)]
. This seems like a failure of cap-lints more than anything else, particularly since this crate could benefit from the feature in question. However, it does seem to be hard for @durka to find a formulation that achieves their goal of guaranteeing divergence and works across all versions of rustc.
- A problem only because the code is used in macros that gets exported to clients, which may have
-
https://github.com/rust-lang/rust/issues/38975 – Unreachable expression in void-1.0.2, Rust 1.16
- This doesn’t seem like a problem. It’s a contained lint error that won’t affect clients, and the existence of this crate can be considered a feature request for precisely the changes we have made. =)
Reviewing this list, I think there are two bugs, and we should probably just fix those, but most of the remaining impact seems all right. It’d be nice though to find a way for @durka to express the pattern they are looking for that checks without warnings on stable stable/nightly, I guess?
I do think in retrospect we didn’t spend enough energy evaluating the impact of this change. It’d be good to have phased it in more gently, at minimum by contacting crate owners. I think this is my fault as the reviewer of the PR for not double-checking that we had run crater and so forth.
That said, there is one thing that I am worried about. Some will recall this prior discussion about the best way to think about uninhabited types and mem::uninitialized
. In that discussion, interestingly, we came to some conclusions that seem somewhat at odds with the current exhaustiveness checking changes. In particular, in that discussion, we talked about whether it ever made sense to have a value of type &!
, and under what conditions that could be considered UB. The general consensus there was that it only became UB if the type was dereferenced – which implies that unsafe code CAN create values of type &!
(say) so long as that reference never escapes to safe code and is never dereferenced by the unsafe code.
However, in nightly today, &!
is considered uninhabited (as is &Void
), which means that code like this typechecks:
enum Void { }
fn foo(x: Result<i32, &Void>) {
let Ok(x) = x;
}
This seems wrong to me given the tentative outcome of the unsafe-code-guidelines discussion discussion above, no? It also seems to be a backwards incompatibility hazard for crates that were using *const Void
to represent void*
pointers, even if they ought not to have been doing that. (But, given the outcome of the UCG discussion, this doesn’t seem wrong, though it’s not what I would do.)