From ffefb13443e5906714d82257c582dd69b385eeea Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 11 Feb 2025 20:57:02 +0000 Subject: [PATCH 1/2] Always perform discr read for never pattern in EUV --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 4 ++++ .../always-read-in-closure-capture.rs | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/ui/rfcs/rfc-0000-never_patterns/always-read-in-closure-capture.rs diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index d37c68f82fb2c..f600325f14599 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -943,6 +943,10 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; let bk = ty::BorrowKind::from_mutbl(mutability); self.delegate.borrow_mut().borrow(place, discr_place.hir_id, bk); + } else if let PatKind::Never = pat.kind { + // A `!` pattern always counts as an immutable read of the discriminant, + // even in an irrefutable pattern. + self.delegate.borrow_mut().borrow(place, discr_place.hir_id, BorrowKind::Immutable); } Ok(()) diff --git a/tests/ui/rfcs/rfc-0000-never_patterns/always-read-in-closure-capture.rs b/tests/ui/rfcs/rfc-0000-never_patterns/always-read-in-closure-capture.rs new file mode 100644 index 0000000000000..3c4cd57ec24b5 --- /dev/null +++ b/tests/ui/rfcs/rfc-0000-never_patterns/always-read-in-closure-capture.rs @@ -0,0 +1,16 @@ +//@ check-pass + +// Make sure that the closure captures `s` so it can perform a read of `s`. + +#![feature(never_patterns)] +#![allow(incomplete_features)] + +enum Void {} + +fn by_value(s: Void) { + move || { + let ! = s; + }; +} + +fn main() {} From 5a76304db63cf7e42908dde599205ddd0cb56f53 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 11 Feb 2025 21:05:52 +0000 Subject: [PATCH 2/2] Nits --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 110 ++++++++++-------- compiler/rustc_hir_typeck/src/upvar.rs | 7 +- 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index f600325f14599..860e619be7187 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -892,61 +892,75 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx let tcx = self.cx.tcx(); self.cat_pattern(discr_place.clone(), pat, &mut |place, pat| { - if let PatKind::Binding(_, canonical_id, ..) = pat.kind { - debug!("walk_pat: binding place={:?} pat={:?}", place, pat); - if let Some(bm) = - self.cx.typeck_results().extract_binding_mode(tcx.sess, pat.hir_id, pat.span) - { - debug!("walk_pat: pat.hir_id={:?} bm={:?}", pat.hir_id, bm); - - // pat_ty: the type of the binding being produced. - let pat_ty = self.node_ty(pat.hir_id)?; - debug!("walk_pat: pat_ty={:?}", pat_ty); + match pat.kind { + PatKind::Binding(_, canonical_id, ..) => { + debug!("walk_pat: binding place={:?} pat={:?}", place, pat); + if let Some(bm) = self + .cx + .typeck_results() + .extract_binding_mode(tcx.sess, pat.hir_id, pat.span) + { + debug!("walk_pat: pat.hir_id={:?} bm={:?}", pat.hir_id, bm); - let def = Res::Local(canonical_id); - if let Ok(ref binding_place) = self.cat_res(pat.hir_id, pat.span, pat_ty, def) { - self.delegate.borrow_mut().bind(binding_place, binding_place.hir_id); - } + // pat_ty: the type of the binding being produced. + let pat_ty = self.node_ty(pat.hir_id)?; + debug!("walk_pat: pat_ty={:?}", pat_ty); - // Subtle: MIR desugaring introduces immutable borrows for each pattern - // binding when lowering pattern guards to ensure that the guard does not - // modify the scrutinee. - if has_guard { - self.delegate.borrow_mut().borrow( - place, - discr_place.hir_id, - BorrowKind::Immutable, - ); - } + let def = Res::Local(canonical_id); + if let Ok(ref binding_place) = + self.cat_res(pat.hir_id, pat.span, pat_ty, def) + { + self.delegate.borrow_mut().bind(binding_place, binding_place.hir_id); + } - // It is also a borrow or copy/move of the value being matched. - // In a cases of pattern like `let pat = upvar`, don't use the span - // of the pattern, as this just looks confusing, instead use the span - // of the discriminant. - match bm.0 { - hir::ByRef::Yes(m) => { - let bk = ty::BorrowKind::from_mutbl(m); - self.delegate.borrow_mut().borrow(place, discr_place.hir_id, bk); + // Subtle: MIR desugaring introduces immutable borrows for each pattern + // binding when lowering pattern guards to ensure that the guard does not + // modify the scrutinee. + if has_guard { + self.delegate.borrow_mut().borrow( + place, + discr_place.hir_id, + BorrowKind::Immutable, + ); } - hir::ByRef::No => { - debug!("walk_pat binding consuming pat"); - self.consume_or_copy(place, discr_place.hir_id); + + // It is also a borrow or copy/move of the value being matched. + // In a cases of pattern like `let pat = upvar`, don't use the span + // of the pattern, as this just looks confusing, instead use the span + // of the discriminant. + match bm.0 { + hir::ByRef::Yes(m) => { + let bk = ty::BorrowKind::from_mutbl(m); + self.delegate.borrow_mut().borrow(place, discr_place.hir_id, bk); + } + hir::ByRef::No => { + debug!("walk_pat binding consuming pat"); + self.consume_or_copy(place, discr_place.hir_id); + } } } } - } else if let PatKind::Deref(subpattern) = pat.kind { - // A deref pattern is a bit special: the binding mode of its inner bindings - // determines whether to borrow *at the level of the deref pattern* rather than - // borrowing the bound place (since that inner place is inside the temporary that - // stores the result of calling `deref()`/`deref_mut()` so can't be captured). - let mutable = self.cx.typeck_results().pat_has_ref_mut_binding(subpattern); - let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; - let bk = ty::BorrowKind::from_mutbl(mutability); - self.delegate.borrow_mut().borrow(place, discr_place.hir_id, bk); - } else if let PatKind::Never = pat.kind { - // A `!` pattern always counts as an immutable read of the discriminant, - // even in an irrefutable pattern. - self.delegate.borrow_mut().borrow(place, discr_place.hir_id, BorrowKind::Immutable); + PatKind::Deref(subpattern) => { + // A deref pattern is a bit special: the binding mode of its inner bindings + // determines whether to borrow *at the level of the deref pattern* rather than + // borrowing the bound place (since that inner place is inside the temporary that + // stores the result of calling `deref()`/`deref_mut()` so can't be captured). + let mutable = self.cx.typeck_results().pat_has_ref_mut_binding(subpattern); + let mutability = + if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + let bk = ty::BorrowKind::from_mutbl(mutability); + self.delegate.borrow_mut().borrow(place, discr_place.hir_id, bk); + } + PatKind::Never => { + // A `!` pattern always counts as an immutable read of the discriminant, + // even in an irrefutable pattern. + self.delegate.borrow_mut().borrow( + place, + discr_place.hir_id, + BorrowKind::Immutable, + ); + } + _ => {} } Ok(()) diff --git a/compiler/rustc_hir_typeck/src/upvar.rs b/compiler/rustc_hir_typeck/src/upvar.rs index c986429c1b24b..39960ff06f8fd 100644 --- a/compiler/rustc_hir_typeck/src/upvar.rs +++ b/compiler/rustc_hir_typeck/src/upvar.rs @@ -1991,17 +1991,18 @@ struct InferBorrowKind<'tcx> { impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> { fn fake_read( &mut self, - place: &PlaceWithHirId<'tcx>, + place_with_id: &PlaceWithHirId<'tcx>, cause: FakeReadCause, diag_expr_id: HirId, ) { - let PlaceBase::Upvar(_) = place.place.base else { return }; + let PlaceBase::Upvar(_) = place_with_id.place.base else { return }; // We need to restrict Fake Read precision to avoid fake reading unsafe code, // such as deref of a raw pointer. let dummy_capture_kind = ty::UpvarCapture::ByRef(ty::BorrowKind::Immutable); - let (place, _) = restrict_capture_precision(place.place.clone(), dummy_capture_kind); + let (place, _) = + restrict_capture_precision(place_with_id.place.clone(), dummy_capture_kind); let (place, _) = restrict_repr_packed_field_ref_capture(place, dummy_capture_kind); self.fake_reads.push((place, cause, diag_expr_id));