ignore uninhabited non-exhaustive variant fields

[davidtwco]

Fixes#65157.

This PR modifies the uninhabitedness checking so that the fields of
a non-exhaustive variant (which is not local) are ignored if they are
uninhabited. This is an improvement over the previous behaviour which
considered all non-local non-exhaustive variants useful because
unreachable patterns are now detected.

r? @arielb1
cc @varkor

[davidtwco]

I'm not sure if this is the best way to fix this, so I'm happy to take an alternate approach if anyone has a better solution.

[Nadrieril]

Yeah, I'd think there must be a better solution.
What was the original reason that tuple variants with non-exhaustive fields should be treated differently ? I couldn't understand why you originally added that line of code

[Centril]

What was the original reason that tuple variants with non-exhaustive fields should be treated differently ? I couldn't understand why you originally added that line of code

The idea is that with #![feature(exhaustive_patterns)], inhabitedness becomes structural rather than taking visibility into account (iirc). That is, for any type (sum or product), the inhabitedness of a type in crate A would leak to the external crate B which would be observable when matching on the type from A inside B. In #60529, the point was to make a #[non_exhaustive] be regarded as inhabited outside of the current crate no matter if it is an enum or a product type (see #60529 (comment)).

[Nadrieril]

So this is about ignoring uninhabitedness when the variant is marked non_exhaustive and not local ? Then I think we should be able to find the code that checks whether a variant is inhabited and tweak that. I can't seem to find what part of the code does that however; I'll keep looking.

[Nadrieril]

Okay I figured it out. I'll take this line as an example:

rust/src/test/ui/rfc-2008-non-exhaustive/uninhabited/patterns.rs

Line 51 in bfc5878

whileletPartiallyInhabitedVariants::Struct{ x, .. } = partially_inhabited_variant(){

That line would be detected as unreachable if the variant wasn't declared non_exhaustive. The reason for this behaviour is that we start with v = [PartiallyInhabitedVariants::Struct { x, .. }], then we specialize it by the PartiallyInhabitedVariants::Struct constructor. Then v = [_] where the wildcard has type !. So it is detected as unreachable.

We can't avoid expanding PartiallyInhabitedVariants::Struct into its fields altogether, otherwise we would incorrectly think the second branch is unreachable here: (Enum taken from #65157):

match Enum::Unit {
Enum::Unit => {}
Enum::Struct { x: true, .. } => {}
Enum::Struct { x: false, .. } => {}
}

So I'd say a good solution might be to specifically ignore the fields of PartiallyInhabitedVariants::Struct that are of an uninhabited type.

Does that make sense ?

PS: @davidtwco would you mind waiting for #65160 to get merged before merging this PR ? Otherwise I'd have to rebase the fix and it will be a pain.

[davidtwco]

Does that make sense ?

I haven’t read this yet, will do later.

PS: @davidtwco would you mind waiting for #65160 to get merged before merging this PR ? Otherwise I'd have to rebase the fix and it will be a pain.

No worries, I just wanted to put something up to get review.

[davidtwco]

Thanks @Nadrieril, I've updated the PR with your suggested approach.

[davidtwco]

r? @varkor

[davidtwco]

@varkor perhaps we should consider this unblocked and proceed with review and merging? - #65160 is already conflicted, this PR is quite small, and this PR is blocking RFC 2008’s stabilisation in #64639.

[Centril]

Sounds reasonable; it does enhance the rationale for @Nadrieril to split their PR into parts. ;)

[varkor]

@bors r+ rollup

[bors]

📌 Commit 7ffbd62 has been approved by varkor