From 436c61266c7e9c4d6b7a57a8a6170700eda68fd0 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 6 Apr 2024 00:29:45 +0200 Subject: [PATCH] Use deep fake borrows for deref patterns --- .../rustc_mir_build/src/build/matches/mod.rs | 17 ++++---- .../rustc_mir_build/src/build/matches/util.rs | 41 ++++++++++--------- .../ui/pattern/deref-patterns/fake_borrows.rs | 14 +++++++ .../deref-patterns/fake_borrows.stderr | 12 ++++++ 4 files changed, 56 insertions(+), 28 deletions(-) create mode 100644 tests/ui/pattern/deref-patterns/fake_borrows.rs create mode 100644 tests/ui/pattern/deref-patterns/fake_borrows.stderr diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index b41523cb9c2..2ab297a47ae 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -375,10 +375,10 @@ fn lower_match_tree<'pat>( match_start_span: Span, match_has_guard: bool, candidates: &mut [&mut Candidate<'pat, 'tcx>], - ) -> Vec<(Place<'tcx>, Local)> { + ) -> Vec<(Place<'tcx>, Local, FakeBorrowKind)> { // The set of places that we are creating fake borrows of. If there are no match guards then // we don't need any fake borrows, so don't track them. - let fake_borrows: Vec<(Place<'tcx>, Local)> = if match_has_guard { + let fake_borrows: Vec<(Place<'tcx>, Local, FakeBorrowKind)> = if match_has_guard { util::collect_fake_borrows( self, candidates, @@ -457,7 +457,7 @@ fn lower_match_arms( scrutinee_span: Span, arm_candidates: Vec<(&'_ Arm<'tcx>, Candidate<'_, 'tcx>)>, outer_source_info: SourceInfo, - fake_borrow_temps: Vec<(Place<'tcx>, Local)>, + fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>, ) -> BlockAnd<()> { let arm_end_blocks: Vec<_> = arm_candidates .into_iter() @@ -541,7 +541,7 @@ fn bind_pattern( &mut self, outer_source_info: SourceInfo, candidate: Candidate<'_, 'tcx>, - fake_borrow_temps: &[(Place<'tcx>, Local)], + fake_borrow_temps: &[(Place<'tcx>, Local, FakeBorrowKind)], scrutinee_span: Span, arm_match_scope: Option<(&Arm<'tcx>, region::Scope)>, storages_alive: bool, @@ -1975,7 +1975,7 @@ fn bind_and_guard_matched_candidate<'pat>( &mut self, candidate: Candidate<'pat, 'tcx>, parent_data: &[PatternExtraData<'tcx>], - fake_borrows: &[(Place<'tcx>, Local)], + fake_borrows: &[(Place<'tcx>, Local, FakeBorrowKind)], scrutinee_span: Span, arm_match_scope: Option<(&Arm<'tcx>, region::Scope)>, schedule_drops: bool, @@ -2105,9 +2105,8 @@ fn bind_and_guard_matched_candidate<'pat>( let re_erased = tcx.lifetimes.re_erased; let scrutinee_source_info = self.source_info(scrutinee_span); - for &(place, temp) in fake_borrows { - let borrow = - Rvalue::Ref(re_erased, BorrowKind::Fake(FakeBorrowKind::Shallow), place); + for &(place, temp, kind) in fake_borrows { + let borrow = Rvalue::Ref(re_erased, BorrowKind::Fake(kind), place); self.cfg.push_assign(block, scrutinee_source_info, Place::from(temp), borrow); } @@ -2130,7 +2129,7 @@ fn bind_and_guard_matched_candidate<'pat>( let guard_frame = self.guard_context.pop().unwrap(); debug!("Exiting guard building context with locals: {:?}", guard_frame); - for &(_, temp) in fake_borrows { + for &(_, temp, _) in fake_borrows { let cause = FakeReadCause::ForMatchGuard; self.cfg.push_fake_read(post_guard_block, guard_end, cause, Place::from(temp)); } diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 967b0c44588..2f9390c22a8 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -1,7 +1,7 @@ use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; use crate::build::matches::{Binding, Candidate, FlatPat, MatchPair, TestCase}; use crate::build::Builder; -use rustc_data_structures::fx::FxIndexSet; +use rustc_data_structures::fx::FxIndexMap; use rustc_infer::infer::type_variable::TypeVariableOrigin; use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; @@ -271,7 +271,11 @@ pub(super) struct FakeBorrowCollector<'a, 'b, 'tcx> { /// Base of the scrutinee place. Used to distinguish bindings inside the scrutinee place from /// bindings inside deref patterns. scrutinee_base: PlaceBase, - fake_borrows: FxIndexSet>, + /// Store for each place the kind of borrow to take. In case of conflicts, we take the strongest + /// borrow (i.e. Deep > Shallow). + /// Invariant: for any place in `fake_borrows`, all the prefixes of this place that are + /// dereferences are also borrowed with the same of stronger borrow kind. + fake_borrows: FxIndexMap, FakeBorrowKind>, } /// Determine the set of places that have to be stable across match guards. @@ -314,9 +318,9 @@ pub(super) fn collect_fake_borrows<'tcx>( candidates: &[&mut Candidate<'_, 'tcx>], temp_span: Span, scrutinee_base: PlaceBase, -) -> Vec<(Place<'tcx>, Local)> { +) -> Vec<(Place<'tcx>, Local, FakeBorrowKind)> { let mut collector = - FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexSet::default() }; + FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexMap::default() }; for candidate in candidates.iter() { collector.visit_candidate(candidate); } @@ -325,40 +329,40 @@ pub(super) fn collect_fake_borrows<'tcx>( let tcx = cx.tcx; fake_borrows .iter() - .copied() - .map(|matched_place| { + .map(|(matched_place, borrow_kind)| { let fake_borrow_deref_ty = matched_place.ty(&cx.local_decls, tcx).ty; let fake_borrow_ty = Ty::new_imm_ref(tcx, tcx.lifetimes.re_erased, fake_borrow_deref_ty); let mut fake_borrow_temp = LocalDecl::new(fake_borrow_ty, temp_span); fake_borrow_temp.local_info = ClearCrossCrate::Set(Box::new(LocalInfo::FakeBorrow)); let fake_borrow_temp = cx.local_decls.push(fake_borrow_temp); - (matched_place, fake_borrow_temp) + (*matched_place, fake_borrow_temp, *borrow_kind) }) .collect() } impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { // Fake borrow this place and its dereference prefixes. - fn fake_borrow(&mut self, place: Place<'tcx>) { - let new = self.fake_borrows.insert(place); - if !new { + fn fake_borrow(&mut self, place: Place<'tcx>, kind: FakeBorrowKind) { + if self.fake_borrows.get(&place).is_some_and(|k| *k >= kind) { return; } + self.fake_borrows.insert(place, kind); // Also fake borrow the prefixes of any fake borrow. - self.fake_borrow_deref_prefixes(place); + self.fake_borrow_deref_prefixes(place, kind); } // Fake borrow the prefixes of this place that are dereferences. - fn fake_borrow_deref_prefixes(&mut self, place: Place<'tcx>) { + fn fake_borrow_deref_prefixes(&mut self, place: Place<'tcx>, kind: FakeBorrowKind) { for (place_ref, elem) in place.as_ref().iter_projections().rev() { if let ProjectionElem::Deref = elem { // Insert a shallow borrow after a deref. For other projections the borrow of // `place_ref` will conflict with any mutation of `place.base`. - let new = self.fake_borrows.insert(place_ref.to_place(self.cx.tcx)); - if !new { + let place = place_ref.to_place(self.cx.tcx); + if self.fake_borrows.get(&place).is_some_and(|k| *k >= kind) { return; } + self.fake_borrows.insert(place, kind); } } } @@ -399,15 +403,14 @@ fn visit_match_pair(&mut self, match_pair: &MatchPair<'_, 'tcx>) { // // UB because we reached the unreachable. // } // ``` - // FIXME(deref_patterns): Hence we fake borrow using a non-shallow borrow. + // Hence we fake borrow using a deep borrow. if let Some(place) = match_pair.place { - // FIXME(deref_patterns): use a non-shallow borrow. - self.fake_borrow(place); + self.fake_borrow(place, FakeBorrowKind::Deep); } } else { // Insert a Shallow borrow of any place that is switched on. if let Some(place) = match_pair.place { - self.fake_borrow(place); + self.fake_borrow(place, FakeBorrowKind::Shallow); } for subpair in &match_pair.subpairs { @@ -447,7 +450,7 @@ fn visit_binding(&mut self, Binding { source, .. }: &Binding<'tcx>) { // _ if { u = true; false } => (), // x => (), // } - self.fake_borrow_deref_prefixes(*source); + self.fake_borrow_deref_prefixes(*source, FakeBorrowKind::Shallow); } } diff --git a/tests/ui/pattern/deref-patterns/fake_borrows.rs b/tests/ui/pattern/deref-patterns/fake_borrows.rs new file mode 100644 index 00000000000..35fa9cbf7d8 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/fake_borrows.rs @@ -0,0 +1,14 @@ +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +#[rustfmt::skip] +fn main() { + let mut b = Box::new(false); + match b { + deref!(true) => {} + _ if { *b = true; false } => {} + //~^ ERROR cannot assign `*b` in match guard + deref!(false) => {} + _ => {}, + } +} diff --git a/tests/ui/pattern/deref-patterns/fake_borrows.stderr b/tests/ui/pattern/deref-patterns/fake_borrows.stderr new file mode 100644 index 00000000000..6a591e6416c --- /dev/null +++ b/tests/ui/pattern/deref-patterns/fake_borrows.stderr @@ -0,0 +1,12 @@ +error[E0510]: cannot assign `*b` in match guard + --> $DIR/fake_borrows.rs:9:16 + | +LL | match b { + | - value is immutable in match guard +LL | deref!(true) => {} +LL | _ if { *b = true; false } => {} + | ^^^^^^^^^ cannot assign + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0510`.