diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index ad34994526ed..9e9b511b9b23 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -818,14 +818,12 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx /// The core driver for walking a pattern /// /// This should mirror how pattern-matching gets lowered to MIR, as - /// otherwise lowering will ICE when trying to resolve the upvars. + /// otherwise said lowering will ICE when trying to resolve the upvars. /// /// However, it is okay to approximate it here by doing *more* accesses than /// the actual MIR builder will, which is useful when some checks are too - /// cumbersome to perform here. For example, if after typeck it becomes - /// clear that only one variant of an enum is inhabited, and therefore a - /// read of the discriminant is not necessary, `walk_pat` will have - /// over-approximated the necessary upvar capture granularity. + /// cumbersome to perform here, because e.g. they require more typeck results + /// than available. /// /// Do note that discrepancies like these do still create obscure corners /// in the semantics of the language, and should be avoided if possible. @@ -1852,26 +1850,13 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx } /// Checks whether a type has multiple variants, and therefore, whether a - /// read of the discriminant might be necessary. Note that the actual MIR - /// builder code does a more specific check, filtering out variants that - /// happen to be uninhabited. - /// - /// Here, it is not practical to perform such a check, because inhabitedness - /// queries require typeck results, and typeck requires closure capture analysis. - /// - /// Moreover, the language is moving towards uninhabited variants still semantically - /// causing a discriminant read, so we *shouldn't* perform any such check. - /// - /// FIXME(never_patterns): update this comment once the aforementioned MIR builder - /// code is changed to be insensitive to inhhabitedness. + /// read of the discriminant might be necessary. #[instrument(skip(self, span), level = "debug")] fn is_multivariant_adt(&self, ty: Ty<'tcx>, span: Span) -> bool { if let ty::Adt(def, _) = self.cx.structurally_resolve_type(span, ty).kind() { - // Note that if a non-exhaustive SingleVariant is defined in another crate, we need - // to assume that more cases will be added to the variant in the future. This mean - // that we should handle non-exhaustive SingleVariant the same way we would handle - // a MultiVariant. - def.variants().len() > 1 || def.variant_list_has_applicable_non_exhaustive() + // We treat non-exhaustive enums the same independent of the crate they are + // defined in, to avoid differences in the operational semantics between crates. + def.variants().len() > 1 || def.is_variant_list_non_exhaustive() } else { false } diff --git a/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.rs b/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.rs index 5b7259c6c2cc..f47d70b52f20 100644 --- a/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.rs +++ b/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.rs @@ -28,12 +28,6 @@ fn main() { let _b = || { match l1 { L1::A => () } }; //~^ ERROR: non-exhaustive patterns: `L1::B` not covered [E0004] - // l2 should not be captured as it is a non-exhaustive SingleVariant - // defined in this crate - let _c = || { match l2 { L2::C => (), _ => () } }; - let mut mut_l2 = l2; - _c(); - // E1 is not visibly uninhabited from here let (e1, e2, e3, e4) = bar(); let _d = || { match e1 {} }; @@ -42,8 +36,14 @@ fn main() { //~^ ERROR: non-exhaustive patterns: `_` not covered [E0004] let _f = || { match e2 { E2::A => (), E2::B => (), _ => () } }; - // e3 should be captured as it is a non-exhaustive SingleVariant - // defined in another crate + // non-exhaustive enums should always be captured, regardless if they + // are defined in the current crate: + let _c = || { match l2 { L2::C => (), _ => () } }; + let mut mut_l2 = l2; + //~^ ERROR: cannot move out of `l2` because it is borrowed + _c(); + + // ...or in another crate: let _g = || { match e3 { E3::C => (), _ => () } }; let mut mut_e3 = e3; //~^ ERROR: cannot move out of `e3` because it is borrowed diff --git a/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.stderr b/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.stderr index 99d33b05429e..e34d1889803a 100644 --- a/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.stderr +++ b/tests/ui/closures/2229_closure_analysis/match/non-exhaustive-match.stderr @@ -16,7 +16,7 @@ LL | let _b = || { match l1 { L1::A => (), L1::B => todo!() } }; | ++++++++++++++++++ error[E0004]: non-exhaustive patterns: type `E1` is non-empty - --> $DIR/non-exhaustive-match.rs:39:25 + --> $DIR/non-exhaustive-match.rs:33:25 | LL | let _d = || { match e1 {} }; | ^^ @@ -35,7 +35,7 @@ LL ~ } }; | error[E0004]: non-exhaustive patterns: `_` not covered - --> $DIR/non-exhaustive-match.rs:41:25 + --> $DIR/non-exhaustive-match.rs:35:25 | LL | let _e = || { match e2 { E2::A => (), E2::B => () } }; | ^^ pattern `_` not covered @@ -52,6 +52,19 @@ help: ensure that all possible cases are being handled by adding a match arm wit LL | let _e = || { match e2 { E2::A => (), E2::B => (), _ => todo!() } }; | ++++++++++++++ +error[E0505]: cannot move out of `l2` because it is borrowed + --> $DIR/non-exhaustive-match.rs:42:22 + | +LL | let _c = || { match l2 { L2::C => (), _ => () } }; + | -- -- borrow occurs due to use in closure + | | + | borrow of `l2` occurs here +LL | let mut mut_l2 = l2; + | ^^ move out of `l2` occurs here +LL | +LL | _c(); + | -- borrow later used here + error[E0505]: cannot move out of `e3` because it is borrowed --> $DIR/non-exhaustive-match.rs:48:22 | @@ -65,7 +78,7 @@ LL | LL | _g(); | -- borrow later used here -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors Some errors have detailed explanations: E0004, E0505. For more information about an error, try `rustc --explain E0004`. diff --git a/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.rs b/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.rs index 1c178bd3f684..348d558dd980 100644 --- a/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.rs +++ b/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.rs @@ -1,3 +1,8 @@ +// Make sure that #[non_exhaustive] cannot cause drop order to depend on which +// crate the code is in. +// +// See rust-lang/rust#147722 +// //@ edition:2021 //@ run-pass //@ check-run-results diff --git a/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.run.stdout b/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.run.stdout index e0d83dd99549..3fa346d87b07 100644 --- a/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.run.stdout +++ b/tests/ui/closures/2229_closure_analysis/match/partial-move-drop-order.run.stdout @@ -12,8 +12,8 @@ dropping b non exhaustive: before assign -dropping a after assign +dropping a dropping b external non exhaustive: diff --git a/tests/ui/closures/2229_closure_analysis/match/partial-move.rs b/tests/ui/closures/2229_closure_analysis/match/partial-move.rs index 62d1b362cb83..7bd435bd1856 100644 --- a/tests/ui/closures/2229_closure_analysis/match/partial-move.rs +++ b/tests/ui/closures/2229_closure_analysis/match/partial-move.rs @@ -78,10 +78,9 @@ pub fn test_two_variants(x: TwoVariants) -> impl FnOnce() { } } -// ...and single-variant, non-exhaustive enums *should* behave as if they had multiple variants +// ...and single-variant, non-exhaustive enums behave as if they had multiple variants pub fn test_non_exhaustive1(x: NonExhaustive) -> impl FnOnce() { || { - //~^ ERROR: closure may outlive the current function, but it borrows `x.0` match x { NonExhaustive::A(a, b) => { drop((a, b)); @@ -94,7 +93,6 @@ pub fn test_non_exhaustive1(x: NonExhaustive) -> impl FnOnce() { // (again, wildcard branch or not) pub fn test_non_exhaustive2(x: NonExhaustive) -> impl FnOnce() { || { - //~^ ERROR: closure may outlive the current function, but it borrows `x.0` match x { NonExhaustive::A(a, b) => { drop((a, b)); diff --git a/tests/ui/closures/2229_closure_analysis/match/partial-move.stderr b/tests/ui/closures/2229_closure_analysis/match/partial-move.stderr index 36adcc6717ed..09f9adf95d5b 100644 --- a/tests/ui/closures/2229_closure_analysis/match/partial-move.stderr +++ b/tests/ui/closures/2229_closure_analysis/match/partial-move.stderr @@ -70,54 +70,6 @@ help: to force the closure to take ownership of `x.0` (and any other referenced LL | move || { | ++++ -error[E0373]: closure may outlive the current function, but it borrows `x.0`, which is owned by the current function - --> $DIR/partial-move.rs:83:5 - | -LL | || { - | ^^ may outlive borrowed value `x.0` -LL | -LL | match x { - | - `x.0` is borrowed here - | -note: closure is returned here - --> $DIR/partial-move.rs:83:5 - | -LL | / || { -LL | | -LL | | match x { -LL | | NonExhaustive::A(a, b) => { -... | -LL | | } - | |_____^ -help: to force the closure to take ownership of `x.0` (and any other referenced variables), use the `move` keyword - | -LL | move || { - | ++++ - -error[E0373]: closure may outlive the current function, but it borrows `x.0`, which is owned by the current function - --> $DIR/partial-move.rs:96:5 - | -LL | || { - | ^^ may outlive borrowed value `x.0` -LL | -LL | match x { - | - `x.0` is borrowed here - | -note: closure is returned here - --> $DIR/partial-move.rs:96:5 - | -LL | / || { -LL | | -LL | | match x { -LL | | NonExhaustive::A(a, b) => { -... | -LL | | } - | |_____^ -help: to force the closure to take ownership of `x.0` (and any other referenced variables), use the `move` keyword - | -LL | move || { - | ++++ - -error: aborting due to 5 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0373`.