From c623319a30d50d88091120fe5ac9354572d56662 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Mar 2024 03:29:52 +0100 Subject: [PATCH 01/10] Lower deref patterns to MIR This handles using deref patterns to choose the correct match arm. This does not handle bindings or guards. Co-authored-by: Deadbeef --- .../src/mem_categorization.rs | 8 +- .../rustc_mir_build/src/build/matches/mod.rs | 7 ++ .../rustc_mir_build/src/build/matches/test.rs | 88 +++++++++++++------ .../rustc_mir_build/src/build/matches/util.rs | 15 ++-- tests/ui/pattern/deref-patterns/bindings.rs | 37 ++++++++ tests/ui/pattern/deref-patterns/branch.rs | 40 +++++++++ .../cant_move_out_of_pattern.rs | 24 +++++ .../cant_move_out_of_pattern.stderr | 27 ++++++ tests/ui/pattern/deref-patterns/typeck.rs | 16 ++-- .../ui/pattern/deref-patterns/typeck_fail.rs | 17 ++++ .../pattern/deref-patterns/typeck_fail.stderr | 19 ++++ 11 files changed, 259 insertions(+), 39 deletions(-) create mode 100644 tests/ui/pattern/deref-patterns/bindings.rs create mode 100644 tests/ui/pattern/deref-patterns/branch.rs create mode 100644 tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs create mode 100644 tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr create mode 100644 tests/ui/pattern/deref-patterns/typeck_fail.rs create mode 100644 tests/ui/pattern/deref-patterns/typeck_fail.stderr diff --git a/compiler/rustc_hir_typeck/src/mem_categorization.rs b/compiler/rustc_hir_typeck/src/mem_categorization.rs index 859877962fe78..15f802560fd35 100644 --- a/compiler/rustc_hir_typeck/src/mem_categorization.rs +++ b/compiler/rustc_hir_typeck/src/mem_categorization.rs @@ -711,13 +711,19 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> { self.cat_pattern_(place_with_id, subpat, op)?; } - PatKind::Box(subpat) | PatKind::Ref(subpat, _) | PatKind::Deref(subpat) => { + PatKind::Box(subpat) | PatKind::Ref(subpat, _) => { // box p1, &p1, &mut p1. we can ignore the mutability of // PatKind::Ref since that information is already contained // in the type. let subplace = self.cat_deref(pat, place_with_id)?; self.cat_pattern_(subplace, subpat, op)?; } + PatKind::Deref(subpat) => { + let ty = self.pat_ty_adjusted(subpat)?; + // A deref pattern generates a temporary. + let place = self.cat_rvalue(pat.hir_id, ty); + self.cat_pattern_(place, subpat, op)?; + } PatKind::Slice(before, ref slice, after) => { let Some(element_ty) = place_with_id.place.ty().builtin_index() else { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 9730473c4288f..f16dddeaa6813 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1163,6 +1163,7 @@ enum TestCase<'pat, 'tcx> { Constant { value: mir::Const<'tcx> }, Range(&'pat PatRange<'tcx>), Slice { len: usize, variable_length: bool }, + Deref { temp: Place<'tcx> }, Or { pats: Box<[FlatPat<'pat, 'tcx>]> }, } @@ -1222,6 +1223,12 @@ enum TestKind<'tcx> { /// Test that the length of the slice is equal to `len`. Len { len: u64, op: BinOp }, + + /// Call `Deref::deref` on the value. + Deref { + /// Temporary to store the result of `deref()`. + temp: Place<'tcx>, + }, } /// A test to perform to determine which [`Candidate`] matches a value. diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 690879b94885a..f6827fd7c5256 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -42,6 +42,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { TestKind::Len { len: len as u64, op } } + TestCase::Deref { temp } => TestKind::Deref { temp }, + TestCase::Or { .. } => bug!("or-patterns should have already been handled"), TestCase::Irrefutable { .. } => span_bug!( @@ -143,35 +145,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } let re_erased = tcx.lifetimes.re_erased; - let ref_string = self.temp(Ty::new_imm_ref(tcx, re_erased, ty), test.span); let ref_str_ty = Ty::new_imm_ref(tcx, re_erased, tcx.types.str_); let ref_str = self.temp(ref_str_ty, test.span); - let deref = tcx.require_lang_item(LangItem::Deref, None); - let method = trait_method(tcx, deref, sym::deref, [ty]); let eq_block = self.cfg.start_new_block(); - self.cfg.push_assign( - block, - source_info, - ref_string, - Rvalue::Ref(re_erased, BorrowKind::Shared, place), - ); - self.cfg.terminate( - block, - source_info, - TerminatorKind::Call { - func: Operand::Constant(Box::new(ConstOperand { - span: test.span, - user_ty: None, - const_: method, - })), - args: vec![Spanned { node: Operand::Move(ref_string), span: DUMMY_SP }], - destination: ref_str, - target: Some(eq_block), - unwind: UnwindAction::Continue, - call_source: CallSource::Misc, - fn_span: source_info.span, - }, - ); + // `let ref_str: &str = ::deref(&place);` + self.call_deref(block, eq_block, place, ty, ref_str, test.span); self.non_scalar_compare( eq_block, success_block, @@ -270,9 +248,57 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Operand::Move(expected), ); } + + TestKind::Deref { temp } => { + let ty = place_ty.ty; + let target = target_block(TestBranch::Success); + self.call_deref(block, target, place, ty, temp, test.span); + } } } + /// Perform `let temp = ::deref(&place)`. + pub(super) fn call_deref( + &mut self, + block: BasicBlock, + target_block: BasicBlock, + place: Place<'tcx>, + ty: Ty<'tcx>, + temp: Place<'tcx>, + span: Span, + ) { + let source_info = self.source_info(span); + let re_erased = self.tcx.lifetimes.re_erased; + let deref = self.tcx.require_lang_item(LangItem::Deref, None); + let method = trait_method(self.tcx, deref, sym::deref, [ty]); + let ref_src = self.temp(Ty::new_imm_ref(self.tcx, re_erased, ty), span); + // `let ref_src = &src_place;` + self.cfg.push_assign( + block, + source_info, + ref_src, + Rvalue::Ref(re_erased, BorrowKind::Shared, place), + ); + // `let temp = ::deref(ref_src);` + self.cfg.terminate( + block, + source_info, + TerminatorKind::Call { + func: Operand::Constant(Box::new(ConstOperand { + span, + user_ty: None, + const_: method, + })), + args: vec![Spanned { node: Operand::Move(ref_src), span }], + destination: temp, + target: Some(target_block), + unwind: UnwindAction::Continue, + call_source: CallSource::Misc, + fn_span: source_info.span, + }, + ); + } + /// Compare using the provided built-in comparison operator fn compare( &mut self, @@ -660,13 +686,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + (TestKind::Deref { temp: test_temp }, TestCase::Deref { temp }) + if test_temp == temp => + { + fully_matched = true; + Some(TestBranch::Success) + } + ( TestKind::Switch { .. } | TestKind::SwitchInt { .. } | TestKind::If | TestKind::Len { .. } | TestKind::Range { .. } - | TestKind::Eq { .. }, + | TestKind::Eq { .. } + | TestKind::Deref { .. }, _, ) => { fully_matched = false; diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index d6376b7b0dcac..4d8248c0e395f 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -5,8 +5,8 @@ use rustc_data_structures::fx::FxIndexSet; use rustc_infer::infer::type_variable::TypeVariableOrigin; use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; -use rustc_middle::ty; use rustc_middle::ty::TypeVisitableExt; +use rustc_middle::ty::{self, Ty}; impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn field_match_pairs<'pat>( @@ -249,10 +249,15 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { default_irrefutable() } - PatKind::DerefPattern { .. } => { - // FIXME(deref_patterns) - // Treat it like a wildcard for now. - default_irrefutable() + PatKind::DerefPattern { ref subpattern } => { + // Create a new temporary for each deref pattern. + // FIXME(deref_patterns): dedup temporaries to avoid multiple `deref()` calls? + let temp = cx.temp( + Ty::new_imm_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty), + pattern.span, + ); + subpairs.push(MatchPair::new(PlaceBuilder::from(temp).deref(), subpattern, cx)); + TestCase::Deref { temp } } }; diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs new file mode 100644 index 0000000000000..75d59c1967ad6 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -0,0 +1,37 @@ +//@ run-pass +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn simple_vec(vec: Vec) -> u32 { + match vec { + deref!([]) => 100, + // FIXME(deref_patterns): fake borrows break guards + // deref!([x]) if x == 4 => x + 4, + deref!([x]) => x, + deref!([1, x]) => x + 200, + deref!(ref slice) => slice.iter().sum(), + _ => 2000, + } +} + +fn nested_vec(vecvec: Vec>) -> u32 { + match vecvec { + deref!([]) => 0, + deref!([deref!([x])]) => x, + deref!([deref!([0, x]) | deref!([1, x])]) => x, + deref!([ref x]) => x.iter().sum(), + deref!([deref!([]), deref!([1, x, y])]) => y - x, + _ => 2000, + } +} + +fn main() { + assert_eq!(simple_vec(vec![1]), 1); + assert_eq!(simple_vec(vec![1, 2]), 202); + assert_eq!(simple_vec(vec![1, 2, 3]), 6); + + assert_eq!(nested_vec(vec![vec![0, 42]]), 42); + assert_eq!(nested_vec(vec![vec![1, 42]]), 42); + assert_eq!(nested_vec(vec![vec![1, 2, 3]]), 6); + assert_eq!(nested_vec(vec![vec![], vec![1, 2, 3]]), 1); +} diff --git a/tests/ui/pattern/deref-patterns/branch.rs b/tests/ui/pattern/deref-patterns/branch.rs new file mode 100644 index 0000000000000..1bac1006d9dc0 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/branch.rs @@ -0,0 +1,40 @@ +//@ run-pass +// Test the execution of deref patterns. +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn branch(vec: Vec) -> u32 { + match vec { + deref!([]) => 0, + deref!([1, _, 3]) => 1, + deref!([2, ..]) => 2, + _ => 1000, + } +} + +fn nested(vec: Vec>) -> u32 { + match vec { + deref!([deref!([]), ..]) => 1, + deref!([deref!([0, ..]), deref!([1, ..])]) => 2, + _ => 1000, + } +} + +fn main() { + assert!(matches!(Vec::::new(), deref!([]))); + assert!(matches!(vec![1], deref!([1]))); + assert!(matches!(&vec![1], deref!([1]))); + assert!(matches!(vec![&1], deref!([1]))); + assert!(matches!(vec![vec![1]], deref!([deref!([1])]))); + + assert_eq!(branch(vec![]), 0); + assert_eq!(branch(vec![1, 2, 3]), 1); + assert_eq!(branch(vec![3, 2, 1]), 1000); + assert_eq!(branch(vec![2]), 2); + assert_eq!(branch(vec![2, 3]), 2); + assert_eq!(branch(vec![3, 2]), 1000); + + assert_eq!(nested(vec![vec![], vec![2]]), 1); + assert_eq!(nested(vec![vec![0], vec![1]]), 2); + assert_eq!(nested(vec![vec![0, 2], vec![1, 2]]), 2); +} diff --git a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs new file mode 100644 index 0000000000000..84b5ec09dc7d8 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs @@ -0,0 +1,24 @@ +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +use std::rc::Rc; + +struct Struct; + +fn cant_move_out_box(b: Box) -> Struct { + match b { + //~^ ERROR: cannot move out of a shared reference + deref!(x) => x, + _ => unreachable!(), + } +} + +fn cant_move_out_rc(rc: Rc) -> Struct { + match rc { + //~^ ERROR: cannot move out of a shared reference + deref!(x) => x, + _ => unreachable!(), + } +} + +fn main() {} diff --git a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr new file mode 100644 index 0000000000000..108db6d9e4b53 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr @@ -0,0 +1,27 @@ +error[E0507]: cannot move out of a shared reference + --> $DIR/cant_move_out_of_pattern.rs:9:11 + | +LL | match b { + | ^ +LL | +LL | deref!(x) => x, + | - + | | + | data moved here + | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + +error[E0507]: cannot move out of a shared reference + --> $DIR/cant_move_out_of_pattern.rs:17:11 + | +LL | match rc { + | ^^ +LL | +LL | deref!(x) => x, + | - + | | + | data moved here + | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0507`. diff --git a/tests/ui/pattern/deref-patterns/typeck.rs b/tests/ui/pattern/deref-patterns/typeck.rs index ead6dcdbaf0f9..f23f7042cd892 100644 --- a/tests/ui/pattern/deref-patterns/typeck.rs +++ b/tests/ui/pattern/deref-patterns/typeck.rs @@ -4,6 +4,8 @@ use std::rc::Rc; +struct Struct; + fn main() { let vec: Vec = Vec::new(); match vec { @@ -22,10 +24,12 @@ fn main() { deref!(1..) => {} _ => {} } - // FIXME(deref_patterns): fails to typecheck because `"foo"` has type &str but deref creates a - // place of type `str`. - // match "foo".to_string() { - // box "foo" => {} - // _ => {} - // } + let _: &Struct = match &Rc::new(Struct) { + deref!(x) => x, + _ => unreachable!(), + }; + let _: &[Struct] = match &Rc::new(vec![Struct]) { + deref!(deref!(x)) => x, + _ => unreachable!(), + }; } diff --git a/tests/ui/pattern/deref-patterns/typeck_fail.rs b/tests/ui/pattern/deref-patterns/typeck_fail.rs new file mode 100644 index 0000000000000..040118449ecca --- /dev/null +++ b/tests/ui/pattern/deref-patterns/typeck_fail.rs @@ -0,0 +1,17 @@ +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn main() { + // FIXME(deref_patterns): fails to typecheck because `"foo"` has type &str but deref creates a + // place of type `str`. + match "foo".to_string() { + deref!("foo") => {} + //~^ ERROR: mismatched types + _ => {} + } + match &"foo".to_string() { + deref!("foo") => {} + //~^ ERROR: mismatched types + _ => {} + } +} diff --git a/tests/ui/pattern/deref-patterns/typeck_fail.stderr b/tests/ui/pattern/deref-patterns/typeck_fail.stderr new file mode 100644 index 0000000000000..1c14802745a0c --- /dev/null +++ b/tests/ui/pattern/deref-patterns/typeck_fail.stderr @@ -0,0 +1,19 @@ +error[E0308]: mismatched types + --> $DIR/typeck_fail.rs:8:16 + | +LL | match "foo".to_string() { + | ----------------- this expression has type `String` +LL | deref!("foo") => {} + | ^^^^^ expected `str`, found `&str` + +error[E0308]: mismatched types + --> $DIR/typeck_fail.rs:13:16 + | +LL | match &"foo".to_string() { + | ------------------ this expression has type `&String` +LL | deref!("foo") => {} + | ^^^^^ expected `str`, found `&str` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 1dabacd059403a7dabf283f4ee02148cb5f69bd4 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 9 Mar 2024 16:10:45 +0100 Subject: [PATCH 02/10] Don't fake borrow inside a deref pattern --- compiler/rustc_mir_build/src/build/matches/mod.rs | 14 ++++++++++++-- tests/ui/pattern/deref-patterns/bindings.rs | 4 ++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index f16dddeaa6813..6094c8e759c0a 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -5,7 +5,7 @@ //! This also includes code for pattern bindings in `let` statements and //! function parameters. -use crate::build::expr::as_place::PlaceBuilder; +use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; use crate::build::scope::DropKind; use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; @@ -438,7 +438,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } if let Some(ref borrows) = fake_borrows { - self.calculate_fake_borrows(borrows, scrutinee_span) + self.calculate_fake_borrows(borrows, scrutinee_place_builder.base(), scrutinee_span) } else { Vec::new() } @@ -1936,6 +1936,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn calculate_fake_borrows<'b>( &mut self, fake_borrows: &'b FxIndexSet>, + scrutinee_base: PlaceBase, temp_span: Span, ) -> Vec<(Place<'tcx>, Local)> { let tcx = self.tcx; @@ -1946,6 +1947,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Insert a Shallow borrow of the prefixes of any fake borrows. for place in fake_borrows { + if let PlaceBase::Local(l) = scrutinee_base + && l != place.local + { + // The base of this place is a temporary created for deref patterns. We don't emit + // fake borrows for these as they are not initialized in all branches. + // FIXME(deref_patterns): is this sound? + continue; + } + let mut cursor = place.projection.as_ref(); while let [proj_base @ .., elem] = cursor { cursor = proj_base; diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index 75d59c1967ad6..c0c8a70dbf0dd 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -5,8 +5,7 @@ fn simple_vec(vec: Vec) -> u32 { match vec { deref!([]) => 100, - // FIXME(deref_patterns): fake borrows break guards - // deref!([x]) if x == 4 => x + 4, + deref!([x]) if x == 4 => x + 4, deref!([x]) => x, deref!([1, x]) => x + 200, deref!(ref slice) => slice.iter().sum(), @@ -29,6 +28,7 @@ fn main() { assert_eq!(simple_vec(vec![1]), 1); assert_eq!(simple_vec(vec![1, 2]), 202); assert_eq!(simple_vec(vec![1, 2, 3]), 6); + assert_eq!(simple_vec(vec![4]), 8); assert_eq!(nested_vec(vec![vec![0, 42]]), 42); assert_eq!(nested_vec(vec![vec![1, 42]]), 42); From 5c4909b8e18eb4fcc7e3b8b6826be5890aa8e529 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 4 Apr 2024 00:25:16 +0200 Subject: [PATCH 03/10] Track mutability of deref patterns --- compiler/rustc_middle/src/thir.rs | 5 +++-- compiler/rustc_middle/src/thir/visit.rs | 2 +- compiler/rustc_mir_build/src/build/matches/mod.rs | 2 +- compiler/rustc_mir_build/src/build/matches/util.rs | 2 +- compiler/rustc_mir_build/src/thir/pattern/mod.rs | 4 +++- compiler/rustc_mir_build/src/thir/print.rs | 2 +- 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 66130a8dde03b..b55fd0a9231d0 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -642,7 +642,7 @@ impl<'tcx> Pat<'tcx> { AscribeUserType { subpattern, .. } | Binding { subpattern: Some(subpattern), .. } | Deref { subpattern } - | DerefPattern { subpattern } + | DerefPattern { subpattern, .. } | InlineConstant { subpattern, .. } => subpattern.walk_(it), Leaf { subpatterns } | Variant { subpatterns, .. } => { subpatterns.iter().for_each(|field| field.pattern.walk_(it)) @@ -760,6 +760,7 @@ pub enum PatKind<'tcx> { /// Deref pattern, written `box P` for now. DerefPattern { subpattern: Box>, + mutability: hir::Mutability, }, /// One of the following: @@ -1163,7 +1164,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> { } write!(f, "{subpattern}") } - PatKind::DerefPattern { ref subpattern } => { + PatKind::DerefPattern { ref subpattern, .. } => { write!(f, "deref!({subpattern})") } PatKind::Constant { value } => write!(f, "{value}"), diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index e42b85530b502..f198881043739 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -229,7 +229,7 @@ pub fn walk_pat<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>( match &pat.kind { AscribeUserType { subpattern, ascription: _ } | Deref { subpattern } - | DerefPattern { subpattern } + | DerefPattern { subpattern, .. } | Binding { subpattern: Some(subpattern), .. } => visitor.visit_pat(subpattern), Binding { .. } | Wild | Never | Error(_) => {} Variant { subpatterns, adt_def: _, args: _, variant_index: _ } | Leaf { subpatterns } => { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 6094c8e759c0a..2ac2c2546dd62 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -938,7 +938,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.visit_primary_bindings(subpattern, pattern_user_ty.deref(), f); } - PatKind::DerefPattern { ref subpattern } => { + PatKind::DerefPattern { ref subpattern, .. } => { self.visit_primary_bindings(subpattern, UserTypeProjections::none(), f); } diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 4d8248c0e395f..3292ab1f1b7e6 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -249,7 +249,7 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { default_irrefutable() } - PatKind::DerefPattern { ref subpattern } => { + PatKind::DerefPattern { ref subpattern, .. } => { // Create a new temporary for each deref pattern. // FIXME(deref_patterns): dedup temporaries to avoid multiple `deref()` calls? let temp = cx.temp( diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index bcb43a0054709..5c016682d8d2d 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -264,7 +264,9 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { } hir::PatKind::Deref(subpattern) => { - PatKind::DerefPattern { subpattern: self.lower_pattern(subpattern) } + let mutable = self.typeck_results.pat_has_ref_mut_binding(subpattern); + let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + PatKind::DerefPattern { subpattern: self.lower_pattern(subpattern), mutability } } hir::PatKind::Ref(subpattern, _) | hir::PatKind::Box(subpattern) => { PatKind::Deref { subpattern: self.lower_pattern(subpattern) } diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index 49e48427b6555..619bfbcf43d3a 100644 --- a/compiler/rustc_mir_build/src/thir/print.rs +++ b/compiler/rustc_mir_build/src/thir/print.rs @@ -688,7 +688,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> { self.print_pat(subpattern, depth_lvl + 2); print_indented!(self, "}", depth_lvl + 1); } - PatKind::DerefPattern { subpattern } => { + PatKind::DerefPattern { subpattern, .. } => { print_indented!(self, "DerefPattern { ", depth_lvl + 1); print_indented!(self, "subpattern:", depth_lvl + 2); self.print_pat(subpattern, depth_lvl + 2); From 377e095371ae99dfdd61d5b50d4a43422b34ca23 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 4 Apr 2024 00:46:35 +0200 Subject: [PATCH 04/10] Allow mutable bindings inside deref patterns --- .../rustc_mir_build/src/build/matches/mod.rs | 7 ++-- .../rustc_mir_build/src/build/matches/test.rs | 35 ++++++++++++++----- .../rustc_mir_build/src/build/matches/util.rs | 6 ++-- tests/ui/pattern/deref-patterns/bindings.rs | 15 ++++++++ 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 2ac2c2546dd62..265d80ed8bc59 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1163,7 +1163,7 @@ enum TestCase<'pat, 'tcx> { Constant { value: mir::Const<'tcx> }, Range(&'pat PatRange<'tcx>), Slice { len: usize, variable_length: bool }, - Deref { temp: Place<'tcx> }, + Deref { temp: Place<'tcx>, mutability: Mutability }, Or { pats: Box<[FlatPat<'pat, 'tcx>]> }, } @@ -1224,10 +1224,11 @@ enum TestKind<'tcx> { /// Test that the length of the slice is equal to `len`. Len { len: u64, op: BinOp }, - /// Call `Deref::deref` on the value. + /// Call `Deref::deref[_mut]` on the value. Deref { - /// Temporary to store the result of `deref()`. + /// Temporary to store the result of `deref()`/`deref_mut()`. temp: Place<'tcx>, + mutability: Mutability, }, } diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index f6827fd7c5256..5dd478aa4224a 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -42,7 +42,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { TestKind::Len { len: len as u64, op } } - TestCase::Deref { temp } => TestKind::Deref { temp }, + TestCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability }, TestCase::Or { .. } => bug!("or-patterns should have already been handled"), @@ -149,7 +149,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let ref_str = self.temp(ref_str_ty, test.span); let eq_block = self.cfg.start_new_block(); // `let ref_str: &str = ::deref(&place);` - self.call_deref(block, eq_block, place, ty, ref_str, test.span); + self.call_deref( + block, + eq_block, + place, + Mutability::Not, + ty, + ref_str, + test.span, + ); self.non_scalar_compare( eq_block, success_block, @@ -249,37 +257,46 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } - TestKind::Deref { temp } => { + TestKind::Deref { temp, mutability } => { let ty = place_ty.ty; let target = target_block(TestBranch::Success); - self.call_deref(block, target, place, ty, temp, test.span); + self.call_deref(block, target, place, mutability, ty, temp, test.span); } } } /// Perform `let temp = ::deref(&place)`. + /// or `let temp = ::deref_mut(&mut place)`. pub(super) fn call_deref( &mut self, block: BasicBlock, target_block: BasicBlock, place: Place<'tcx>, + mutability: Mutability, ty: Ty<'tcx>, temp: Place<'tcx>, span: Span, ) { + let (trait_item, method) = match mutability { + Mutability::Not => (LangItem::Deref, sym::deref), + Mutability::Mut => (LangItem::DerefMut, sym::deref_mut), + }; + let borrow_kind = super::util::ref_pat_borrow_kind(mutability); let source_info = self.source_info(span); let re_erased = self.tcx.lifetimes.re_erased; - let deref = self.tcx.require_lang_item(LangItem::Deref, None); - let method = trait_method(self.tcx, deref, sym::deref, [ty]); - let ref_src = self.temp(Ty::new_imm_ref(self.tcx, re_erased, ty), span); + let trait_item = self.tcx.require_lang_item(trait_item, None); + let method = trait_method(self.tcx, trait_item, method, [ty]); + let ref_src = self.temp(Ty::new_ref(self.tcx, re_erased, ty, mutability), span); // `let ref_src = &src_place;` + // or `let ref_src = &mut src_place;` self.cfg.push_assign( block, source_info, ref_src, - Rvalue::Ref(re_erased, BorrowKind::Shared, place), + Rvalue::Ref(re_erased, borrow_kind, place), ); // `let temp = ::deref(ref_src);` + // or `let temp = ::deref_mut(ref_src);` self.cfg.terminate( block, source_info, @@ -686,7 +703,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - (TestKind::Deref { temp: test_temp }, TestCase::Deref { temp }) + (TestKind::Deref { temp: test_temp, .. }, TestCase::Deref { temp, .. }) if test_temp == temp => { fully_matched = true; diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 3292ab1f1b7e6..8f9fb8beba176 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -249,15 +249,15 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { default_irrefutable() } - PatKind::DerefPattern { ref subpattern, .. } => { + PatKind::DerefPattern { ref subpattern, mutability } => { // Create a new temporary for each deref pattern. // FIXME(deref_patterns): dedup temporaries to avoid multiple `deref()` calls? let temp = cx.temp( - Ty::new_imm_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty), + Ty::new_ref(cx.tcx, cx.tcx.lifetimes.re_erased, subpattern.ty, mutability), pattern.span, ); subpairs.push(MatchPair::new(PlaceBuilder::from(temp).deref(), subpattern, cx)); - TestCase::Deref { temp } + TestCase::Deref { temp, mutability } } }; diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index c0c8a70dbf0dd..4f72058af8fe5 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -24,6 +24,19 @@ fn nested_vec(vecvec: Vec>) -> u32 { } } +fn ref_mut(val: u32) -> u32 { + let mut b = Box::new(0u32); + match &mut b { + deref!(_x) if false => unreachable!(), + deref!(x) => { + *x = val; + } + _ => unreachable!(), + } + let deref!(x) = &b else { unreachable!() }; + *x +} + fn main() { assert_eq!(simple_vec(vec![1]), 1); assert_eq!(simple_vec(vec![1, 2]), 202); @@ -34,4 +47,6 @@ fn main() { assert_eq!(nested_vec(vec![vec![1, 42]]), 42); assert_eq!(nested_vec(vec![vec![1, 2, 3]]), 6); assert_eq!(nested_vec(vec![vec![], vec![1, 2, 3]]), 1); + + assert_eq!(ref_mut(42), 42) } From b55afe475aee1e51196e35845ba1553c96f041cf Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Fri, 5 Apr 2024 23:42:29 +0200 Subject: [PATCH 05/10] Address closure-related review --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 9 ++++++++ .../src/mem_categorization.rs | 4 ++++ .../rustc_middle/src/ty/typeck_results.rs | 2 +- .../pattern/deref-patterns/closure_capture.rs | 21 +++++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/ui/pattern/deref-patterns/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 a0a5a75d3820a..cc42e69f53827 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -750,6 +750,15 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } } + } 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 = mc.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); + delegate.borrow(place, discr_place.hir_id, bk); } })); } diff --git a/compiler/rustc_hir_typeck/src/mem_categorization.rs b/compiler/rustc_hir_typeck/src/mem_categorization.rs index 15f802560fd35..b44c2345933fb 100644 --- a/compiler/rustc_hir_typeck/src/mem_categorization.rs +++ b/compiler/rustc_hir_typeck/src/mem_categorization.rs @@ -719,7 +719,11 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> { self.cat_pattern_(subplace, subpat, op)?; } PatKind::Deref(subpat) => { + let mutable = self.typeck_results.pat_has_ref_mut_binding(subpat); + let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + let re_erased = self.tcx().lifetimes.re_erased; let ty = self.pat_ty_adjusted(subpat)?; + let ty = Ty::new_ref(self.tcx(), re_erased, ty, mutability); // A deref pattern generates a temporary. let place = self.cat_rvalue(pat.hir_id, ty); self.cat_pattern_(place, subpat, op)?; diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs index a28afcc4fb8c2..25bca70f1029e 100644 --- a/compiler/rustc_middle/src/ty/typeck_results.rs +++ b/compiler/rustc_middle/src/ty/typeck_results.rs @@ -451,7 +451,7 @@ impl<'tcx> TypeckResults<'tcx> { /// This is computed from the typeck results since we want to make /// sure to apply any match-ergonomics adjustments, which we cannot /// determine from the HIR alone. - pub fn pat_has_ref_mut_binding(&self, pat: &'tcx hir::Pat<'tcx>) -> bool { + pub fn pat_has_ref_mut_binding(&self, pat: &hir::Pat<'_>) -> bool { let mut has_ref_mut = false; pat.walk(|pat| { if let hir::PatKind::Binding(_, id, _, _) = pat.kind diff --git a/tests/ui/pattern/deref-patterns/closure_capture.rs b/tests/ui/pattern/deref-patterns/closure_capture.rs new file mode 100644 index 0000000000000..fc0ddedac2b78 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/closure_capture.rs @@ -0,0 +1,21 @@ +//@ run-pass +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +fn main() { + let b = Box::new("aaa".to_string()); + let f = || { + let deref!(ref s) = b else { unreachable!() }; + assert_eq!(s.len(), 3); + }; + assert_eq!(b.len(), 3); + f(); + + let mut b = Box::new("aaa".to_string()); + let mut f = || { + let deref!(ref mut s) = b else { unreachable!() }; + s.push_str("aa"); + }; + f(); + assert_eq!(b.len(), 5); +} From 511bd78863052fae0026698ed139774f3334e5b1 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 7 Apr 2024 01:23:39 +0200 Subject: [PATCH 06/10] Rework fake borrow calculation --- compiler/rustc_middle/src/mir/statement.rs | 5 + .../rustc_mir_build/src/build/matches/mod.rs | 117 ++------------- .../rustc_mir_build/src/build/matches/util.rs | 139 ++++++++++++++++-- ...se_edges.full_tested_match.built.after.mir | 12 +- ...e_edges.full_tested_match2.built.after.mir | 12 +- .../match_false_edges.main.built.after.mir | 20 +-- ....constant_eq.SimplifyCfg-initial.after.mir | 24 +-- ...joint_ranges.SimplifyCfg-initial.after.mir | 24 +-- ...guard.CleanupPostBorrowck.panic-abort.diff | 16 +- ...uard.CleanupPostBorrowck.panic-unwind.diff | 16 +- 10 files changed, 206 insertions(+), 179 deletions(-) diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs index 069c8019cb2c8..235298ffcb8bc 100644 --- a/compiler/rustc_middle/src/mir/statement.rs +++ b/compiler/rustc_middle/src/mir/statement.rs @@ -236,6 +236,11 @@ impl<'tcx> PlaceRef<'tcx> { } } + #[inline] + pub fn to_place(&self, tcx: TyCtxt<'tcx>) -> Place<'tcx> { + Place { local: self.local, projection: tcx.mk_place_elems(self.projection) } + } + #[inline] pub fn last_projection(&self) -> Option<(PlaceRef<'tcx>, PlaceElem<'tcx>)> { if let &[ref proj_base @ .., elem] = self.projection { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 265d80ed8bc59..648d0b52b3214 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -5,15 +5,12 @@ //! This also includes code for pattern bindings in `let` statements and //! function parameters. -use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; +use crate::build::expr::as_place::PlaceBuilder; use crate::build::scope::DropKind; use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::build::{GuardFrame, GuardFrameLocal, LocalsForNode}; -use rustc_data_structures::{ - fx::{FxHashSet, FxIndexMap, FxIndexSet}, - stack::ensure_sufficient_stack, -}; +use rustc_data_structures::{fx::FxIndexMap, stack::ensure_sufficient_stack}; use rustc_hir::{BindingMode, ByRef}; use rustc_middle::middle::region; use rustc_middle::mir::{self, *}; @@ -209,7 +206,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// 2. Create the decision tree ([Builder::lower_match_tree]). /// 3. Determine the fake borrows that are needed from the places that were /// matched against and create the required temporaries for them - /// ([Builder::calculate_fake_borrows]). + /// ([util::collect_fake_borrows]). /// 4. Create everything else: the guards and the arms ([Builder::lower_match_arms]). /// /// ## False edges @@ -379,11 +376,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match_has_guard: bool, candidates: &mut [&mut Candidate<'pat, 'tcx>], ) -> Vec<(Place<'tcx>, Local)> { - // 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 = match_has_guard - .then(|| util::FakeBorrowCollector::collect_fake_borrows(self, candidates)); + // 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 { + util::collect_fake_borrows( + self, + candidates, + scrutinee_span, + scrutinee_place_builder.base(), + ) + } else { + Vec::new() + }; // See the doc comment on `match_candidates` for why we have an // otherwise block. Match checking will ensure this is actually @@ -437,11 +441,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }); } - if let Some(ref borrows) = fake_borrows { - self.calculate_fake_borrows(borrows, scrutinee_place_builder.base(), scrutinee_span) - } else { - Vec::new() - } + fake_borrows } /// Lower the bindings, guards and arm bodies of a `match` expression. @@ -1911,91 +1911,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { target_blocks, ); } - - /// Determine the fake borrows that are needed from a set of places that - /// have to be stable across match guards. - /// - /// Returns a list of places that need a fake borrow and the temporary - /// that's used to store the fake borrow. - /// - /// Match exhaustiveness checking is not able to handle the case where the - /// place being matched on is mutated in the guards. We add "fake borrows" - /// to the guards that prevent any mutation of the place being matched. - /// There are a some subtleties: - /// - /// 1. Borrowing `*x` doesn't prevent assigning to `x`. If `x` is a shared - /// reference, the borrow isn't even tracked. As such we have to add fake - /// borrows of any prefixes of a place - /// 2. We don't want `match x { _ => (), }` to conflict with mutable - /// borrows of `x`, so we only add fake borrows for places which are - /// bound or tested by the match. - /// 3. We don't want the fake borrows to conflict with `ref mut` bindings, - /// so we use a special BorrowKind for them. - /// 4. The fake borrows may be of places in inactive variants, so it would - /// be UB to generate code for them. They therefore have to be removed - /// by a MIR pass run after borrow checking. - fn calculate_fake_borrows<'b>( - &mut self, - fake_borrows: &'b FxIndexSet>, - scrutinee_base: PlaceBase, - temp_span: Span, - ) -> Vec<(Place<'tcx>, Local)> { - let tcx = self.tcx; - - debug!("add_fake_borrows fake_borrows = {:?}", fake_borrows); - - let mut all_fake_borrows = Vec::with_capacity(fake_borrows.len()); - - // Insert a Shallow borrow of the prefixes of any fake borrows. - for place in fake_borrows { - if let PlaceBase::Local(l) = scrutinee_base - && l != place.local - { - // The base of this place is a temporary created for deref patterns. We don't emit - // fake borrows for these as they are not initialized in all branches. - // FIXME(deref_patterns): is this sound? - continue; - } - - let mut cursor = place.projection.as_ref(); - while let [proj_base @ .., elem] = cursor { - cursor = proj_base; - - if let ProjectionElem::Deref = elem { - // Insert a shallow borrow after a deref. For other - // projections the borrow of prefix_cursor will - // conflict with any mutation of base. - all_fake_borrows.push(PlaceRef { local: place.local, projection: proj_base }); - } - } - - all_fake_borrows.push(place.as_ref()); - } - - // Deduplicate - let mut dedup = FxHashSet::default(); - all_fake_borrows.retain(|b| dedup.insert(*b)); - - debug!("add_fake_borrows all_fake_borrows = {:?}", all_fake_borrows); - - all_fake_borrows - .into_iter() - .map(|matched_place_ref| { - let matched_place = Place { - local: matched_place_ref.local, - projection: tcx.mk_place_elems(matched_place_ref.projection), - }; - let fake_borrow_deref_ty = matched_place.ty(&self.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 = self.local_decls.push(fake_borrow_temp); - - (matched_place, fake_borrow_temp) - }) - .collect() - } } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 8f9fb8beba176..967b0c44588fa 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -7,6 +7,7 @@ use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; use rustc_middle::ty::TypeVisitableExt; use rustc_middle::ty::{self, Ty}; +use rustc_span::Span; impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn field_match_pairs<'pat>( @@ -267,19 +268,99 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { pub(super) struct FakeBorrowCollector<'a, 'b, 'tcx> { cx: &'a mut Builder<'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>, } +/// Determine the set of places that have to be stable across match guards. +/// +/// Returns a list of places that need a fake borrow along with a local to store it. +/// +/// Match exhaustiveness checking is not able to handle the case where the place being matched on is +/// mutated in the guards. We add "fake borrows" to the guards that prevent any mutation of the +/// place being matched. There are a some subtleties: +/// +/// 1. Borrowing `*x` doesn't prevent assigning to `x`. If `x` is a shared reference, the borrow +/// isn't even tracked. As such we have to add fake borrows of any prefixes of a place. +/// 2. We don't want `match x { (Some(_), _) => (), .. }` to conflict with mutable borrows of `x.1`, so we +/// only add fake borrows for places which are bound or tested by the match. +/// 3. We don't want `match x { Some(_) => (), .. }` to conflict with mutable borrows of `(x as +/// Some).0`, so the borrows are a special shallow borrow that only affects the place and not its +/// projections. +/// ```rust +/// let mut x = (Some(0), true); +/// match x { +/// (Some(_), false) => {} +/// _ if { if let Some(ref mut y) = x.0 { *y += 1 }; true } => {} +/// _ => {} +/// } +/// ``` +/// 4. The fake borrows may be of places in inactive variants, e.g. here we need to fake borrow `x` +/// and `(x as Some).0`, but when we reach the guard `x` may not be `Some`. +/// ```rust +/// let mut x = (Some(Some(0)), true); +/// match x { +/// (Some(Some(_)), false) => {} +/// _ if { if let Some(Some(ref mut y)) = x.0 { *y += 1 }; true } => {} +/// _ => {} +/// } +/// ``` +/// So it would be UB to generate code for the fake borrows. They therefore have to be removed by +/// a MIR pass run after borrow checking. +pub(super) fn collect_fake_borrows<'tcx>( + cx: &mut Builder<'_, 'tcx>, + candidates: &[&mut Candidate<'_, 'tcx>], + temp_span: Span, + scrutinee_base: PlaceBase, +) -> Vec<(Place<'tcx>, Local)> { + let mut collector = + FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexSet::default() }; + for candidate in candidates.iter() { + collector.visit_candidate(candidate); + } + let fake_borrows = collector.fake_borrows; + debug!("add_fake_borrows fake_borrows = {:?}", fake_borrows); + let tcx = cx.tcx; + fake_borrows + .iter() + .copied() + .map(|matched_place| { + 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) + }) + .collect() +} + impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { - pub(super) fn collect_fake_borrows( - cx: &'a mut Builder<'b, 'tcx>, - candidates: &[&mut Candidate<'_, 'tcx>], - ) -> FxIndexSet> { - let mut collector = Self { cx, fake_borrows: FxIndexSet::default() }; - for candidate in candidates.iter() { - collector.visit_candidate(candidate); + // 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 { + return; + } + // Also fake borrow the prefixes of any fake borrow. + self.fake_borrow_deref_prefixes(place); + } + + // Fake borrow the prefixes of this place that are dereferences. + fn fake_borrow_deref_prefixes(&mut self, place: Place<'tcx>) { + 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 { + return; + } + } } - collector.fake_borrows } fn visit_candidate(&mut self, candidate: &Candidate<'_, 'tcx>) { @@ -305,10 +386,28 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { for flat_pat in pats.iter() { self.visit_flat_pat(flat_pat) } + } else if matches!(match_pair.test_case, TestCase::Deref { .. }) { + // The subpairs of a deref pattern are all places relative to the deref temporary, so we + // don't fake borrow them. Problem is, if we only shallowly fake-borrowed + // `match_pair.place`, this would allow: + // ``` + // let mut b = Box::new(false); + // match b { + // deref!(true) => {} // not reached because `*b == false` + // _ if { *b = true; false } => {} // not reached because the guard is `false` + // deref!(false) => {} // not reached because the guard changed it + // // UB because we reached the unreachable. + // } + // ``` + // FIXME(deref_patterns): Hence we fake borrow using a non-shallow borrow. + if let Some(place) = match_pair.place { + // FIXME(deref_patterns): use a non-shallow borrow. + self.fake_borrow(place); + } } else { // Insert a Shallow borrow of any place that is switched on. if let Some(place) = match_pair.place { - self.fake_borrows.insert(place); + self.fake_borrow(place); } for subpair in &match_pair.subpairs { @@ -318,6 +417,14 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { } fn visit_binding(&mut self, Binding { source, .. }: &Binding<'tcx>) { + if let PlaceBase::Local(l) = self.scrutinee_base + && l != source.local + { + // The base of this place is a temporary created for deref patterns. We don't emit fake + // borrows for these as they are not initialized in all branches. + return; + } + // Insert a borrows of prefixes of places that are bound and are // behind a dereference projection. // @@ -334,13 +441,13 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { // y if { y == 1 && (x = &2) == () } => y, // _ => 3, // } - if let Some(i) = source.projection.iter().rposition(|elem| elem == ProjectionElem::Deref) { - let proj_base = &source.projection[..i]; - self.fake_borrows.insert(Place { - local: source.local, - projection: self.cx.tcx.mk_place_elems(proj_base), - }); - } + // + // We don't just fake borrow the whole place because this is allowed: + // match u { + // _ if { u = true; false } => (), + // x => (), + // } + self.fake_borrow_deref_prefixes(*source); } } diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir index 194afdf7dd8a8..0d381ce70f175 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir @@ -4,8 +4,8 @@ fn full_tested_match() -> () { let mut _0: (); let mut _1: (i32, i32); let mut _2: std::option::Option; - let mut _3: isize; - let mut _4: &std::option::Option; + let mut _3: &std::option::Option; + let mut _4: isize; let _5: i32; let _6: &i32; let mut _7: bool; @@ -27,8 +27,8 @@ fn full_tested_match() -> () { StorageLive(_2); _2 = Option::::Some(const 42_i32); PlaceMention(_2); - _3 = discriminant(_2); - switchInt(move _3) -> [0: bb5, 1: bb2, otherwise: bb1]; + _4 = discriminant(_2); + switchInt(move _4) -> [0: bb5, 1: bb2, otherwise: bb1]; } bb1: { @@ -60,7 +60,7 @@ fn full_tested_match() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _4 = &fake _2; + _3 = &fake _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } @@ -71,7 +71,7 @@ fn full_tested_match() -> () { bb9: { StorageDead(_7); - FakeRead(ForMatchGuard, _4); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _6); StorageLive(_5); _5 = ((_2 as Some).0: i32); diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir index ae83075434f7b..d397de66fc470 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir @@ -4,8 +4,8 @@ fn full_tested_match2() -> () { let mut _0: (); let mut _1: (i32, i32); let mut _2: std::option::Option; - let mut _3: isize; - let mut _4: &std::option::Option; + let mut _3: &std::option::Option; + let mut _4: isize; let _5: i32; let _6: &i32; let mut _7: bool; @@ -27,8 +27,8 @@ fn full_tested_match2() -> () { StorageLive(_2); _2 = Option::::Some(const 42_i32); PlaceMention(_2); - _3 = discriminant(_2); - switchInt(move _3) -> [0: bb5, 1: bb2, otherwise: bb1]; + _4 = discriminant(_2); + switchInt(move _4) -> [0: bb5, 1: bb2, otherwise: bb1]; } bb1: { @@ -66,7 +66,7 @@ fn full_tested_match2() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _4 = &fake _2; + _3 = &fake _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } @@ -77,7 +77,7 @@ fn full_tested_match2() -> () { bb9: { StorageDead(_7); - FakeRead(ForMatchGuard, _4); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _6); StorageLive(_5); _5 = ((_2 as Some).0: i32); diff --git a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir index dfa31cfff6b2c..63920cec885ad 100644 --- a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir @@ -4,9 +4,9 @@ fn main() -> () { let mut _0: (); let mut _1: i32; let mut _2: std::option::Option; - let mut _3: isize; + let mut _3: &std::option::Option; let mut _4: isize; - let mut _5: &std::option::Option; + let mut _5: isize; let _6: i32; let _7: &i32; let mut _8: bool; @@ -38,8 +38,8 @@ fn main() -> () { StorageLive(_2); _2 = Option::::Some(const 1_i32); PlaceMention(_2); - _4 = discriminant(_2); - switchInt(move _4) -> [1: bb8, otherwise: bb2]; + _5 = discriminant(_2); + switchInt(move _5) -> [1: bb8, otherwise: bb2]; } bb1: { @@ -52,8 +52,8 @@ fn main() -> () { } bb3: { - _3 = discriminant(_2); - switchInt(move _3) -> [1: bb6, otherwise: bb4]; + _4 = discriminant(_2); + switchInt(move _4) -> [1: bb6, otherwise: bb4]; } bb4: { @@ -87,7 +87,7 @@ fn main() -> () { bb10: { StorageLive(_7); _7 = &((_2 as Some).0: i32); - _5 = &fake _2; + _3 = &fake _2; StorageLive(_8); _8 = guard() -> [return: bb11, unwind: bb24]; } @@ -98,7 +98,7 @@ fn main() -> () { bb12: { StorageDead(_8); - FakeRead(ForMatchGuard, _5); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _7); StorageLive(_6); _6 = ((_2 as Some).0: i32); @@ -129,7 +129,7 @@ fn main() -> () { bb16: { StorageLive(_11); _11 = &((_2 as Some).0: i32); - _5 = &fake _2; + _3 = &fake _2; StorageLive(_12); StorageLive(_13); _13 = (*_11); @@ -143,7 +143,7 @@ fn main() -> () { bb18: { StorageDead(_13); StorageDead(_12); - FakeRead(ForMatchGuard, _5); + FakeRead(ForMatchGuard, _3); FakeRead(ForGuardBinding, _11); StorageLive(_10); _10 = ((_2 as Some).0: i32); diff --git a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir index c3497c6989d77..21ddd39137fb2 100644 --- a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir @@ -7,10 +7,10 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { let mut _3: (&str, bool); let mut _4: &str; let mut _5: bool; - let mut _6: bool; - let mut _7: bool; - let mut _8: &&str; - let mut _9: &bool; + let mut _6: &&str; + let mut _7: &bool; + let mut _8: bool; + let mut _9: bool; let mut _10: bool; bb0: { @@ -23,7 +23,7 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { StorageDead(_5); StorageDead(_4); PlaceMention(_3); - _7 = ::eq((_3.0: &str), const "a") -> [return: bb11, unwind: bb19]; + _9 = ::eq((_3.0: &str), const "a") -> [return: bb11, unwind: bb19]; } bb1: { @@ -52,7 +52,7 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { } bb7: { - _6 = ::eq((_3.0: &str), const "b") -> [return: bb10, unwind: bb19]; + _8 = ::eq((_3.0: &str), const "b") -> [return: bb10, unwind: bb19]; } bb8: { @@ -64,16 +64,16 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { } bb10: { - switchInt(move _6) -> [0: bb1, otherwise: bb8]; + switchInt(move _8) -> [0: bb1, otherwise: bb8]; } bb11: { - switchInt(move _7) -> [0: bb7, otherwise: bb4]; + switchInt(move _9) -> [0: bb7, otherwise: bb4]; } bb12: { - _8 = &fake (_3.0: &str); - _9 = &fake (_3.1: bool); + _6 = &fake (_3.0: &str); + _7 = &fake (_3.1: bool); StorageLive(_10); _10 = const true; switchInt(move _10) -> [0: bb14, otherwise: bb13]; @@ -81,8 +81,8 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { bb13: { StorageDead(_10); - FakeRead(ForMatchGuard, _8); - FakeRead(ForMatchGuard, _9); + FakeRead(ForMatchGuard, _6); + FakeRead(ForMatchGuard, _7); _0 = const 1_u32; goto -> bb18; } diff --git a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir index 4a1e4fb9ec56f..6068cef8fbcb0 100644 --- a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir @@ -4,17 +4,17 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { debug x => _1; debug b => _2; let mut _0: u32; - let mut _3: bool; + let mut _3: &i32; let mut _4: bool; let mut _5: bool; let mut _6: bool; - let mut _7: &i32; + let mut _7: bool; let mut _8: bool; bb0: { PlaceMention(_1); - _5 = Le(const 0_i32, _1); - switchInt(move _5) -> [0: bb3, otherwise: bb8]; + _6 = Le(const 0_i32, _1); + switchInt(move _6) -> [0: bb3, otherwise: bb8]; } bb1: { @@ -27,8 +27,8 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { } bb3: { - _3 = Le(const 10_i32, _1); - switchInt(move _3) -> [0: bb5, otherwise: bb7]; + _4 = Le(const 10_i32, _1); + switchInt(move _4) -> [0: bb5, otherwise: bb7]; } bb4: { @@ -44,17 +44,17 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { } bb7: { - _4 = Le(_1, const 20_i32); - switchInt(move _4) -> [0: bb5, otherwise: bb4]; + _5 = Le(_1, const 20_i32); + switchInt(move _5) -> [0: bb5, otherwise: bb4]; } bb8: { - _6 = Lt(_1, const 10_i32); - switchInt(move _6) -> [0: bb3, otherwise: bb2]; + _7 = Lt(_1, const 10_i32); + switchInt(move _7) -> [0: bb3, otherwise: bb2]; } bb9: { - _7 = &fake _1; + _3 = &fake _1; StorageLive(_8); _8 = _2; switchInt(move _8) -> [0: bb11, otherwise: bb10]; @@ -62,7 +62,7 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { bb10: { StorageDead(_8); - FakeRead(ForMatchGuard, _7); + FakeRead(ForMatchGuard, _3); _0 = const 0_u32; goto -> bb14; } diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff index 54da6ee659f95..8feb59ec8747b 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff @@ -5,17 +5,17 @@ debug x => _1; debug c => _2; let mut _0: i32; - let mut _3: isize; - let mut _4: &std::option::Option<&&i32>; + let mut _3: &std::option::Option<&&i32>; + let mut _4: &i32; let mut _5: &&i32; let mut _6: &&&i32; - let mut _7: &i32; + let mut _7: isize; let mut _8: bool; bb0: { PlaceMention(_1); - _3 = discriminant(_1); - switchInt(move _3) -> [1: bb2, otherwise: bb1]; + _7 = discriminant(_1); + switchInt(move _7) -> [1: bb2, otherwise: bb1]; } bb1: { @@ -33,10 +33,10 @@ } bb4: { -- _4 = &fake _1; +- _3 = &fake _1; +- _4 = &fake (*(*((_1 as Some).0: &&i32))); - _5 = &fake (*((_1 as Some).0: &&i32)); - _6 = &fake ((_1 as Some).0: &&i32); -- _7 = &fake (*(*((_1 as Some).0: &&i32))); + nop; + nop; + nop; @@ -48,10 +48,10 @@ bb5: { StorageDead(_8); +- FakeRead(ForMatchGuard, _3); - FakeRead(ForMatchGuard, _4); - FakeRead(ForMatchGuard, _5); - FakeRead(ForMatchGuard, _6); -- FakeRead(ForMatchGuard, _7); + nop; + nop; + nop; diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff index 54da6ee659f95..8feb59ec8747b 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff @@ -5,17 +5,17 @@ debug x => _1; debug c => _2; let mut _0: i32; - let mut _3: isize; - let mut _4: &std::option::Option<&&i32>; + let mut _3: &std::option::Option<&&i32>; + let mut _4: &i32; let mut _5: &&i32; let mut _6: &&&i32; - let mut _7: &i32; + let mut _7: isize; let mut _8: bool; bb0: { PlaceMention(_1); - _3 = discriminant(_1); - switchInt(move _3) -> [1: bb2, otherwise: bb1]; + _7 = discriminant(_1); + switchInt(move _7) -> [1: bb2, otherwise: bb1]; } bb1: { @@ -33,10 +33,10 @@ } bb4: { -- _4 = &fake _1; +- _3 = &fake _1; +- _4 = &fake (*(*((_1 as Some).0: &&i32))); - _5 = &fake (*((_1 as Some).0: &&i32)); - _6 = &fake ((_1 as Some).0: &&i32); -- _7 = &fake (*(*((_1 as Some).0: &&i32))); + nop; + nop; + nop; @@ -48,10 +48,10 @@ bb5: { StorageDead(_8); +- FakeRead(ForMatchGuard, _3); - FakeRead(ForMatchGuard, _4); - FakeRead(ForMatchGuard, _5); - FakeRead(ForMatchGuard, _6); -- FakeRead(ForMatchGuard, _7); + nop; + nop; + nop; From 50531806ee4c77be601ecf2fbdac371288770e17 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 7 Apr 2024 00:30:28 +0200 Subject: [PATCH 07/10] Add a non-shallow fake borrow --- compiler/rustc_borrowck/src/borrow_set.rs | 3 +- .../src/diagnostics/conflict_errors.rs | 29 +++++--- .../rustc_borrowck/src/diagnostics/mod.rs | 2 +- compiler/rustc_borrowck/src/lib.rs | 29 ++++---- .../rustc_borrowck/src/places_conflict.rs | 6 +- .../src/polonius/loan_invalidations.rs | 19 +++-- .../src/transform/check_consts/check.rs | 4 +- .../src/transform/check_consts/resolver.rs | 2 +- .../src/transform/validate.rs | 2 +- compiler/rustc_middle/src/mir/pretty.rs | 3 +- compiler/rustc_middle/src/mir/statement.rs | 4 +- compiler/rustc_middle/src/mir/syntax.rs | 73 ++++++++++++++++--- compiler/rustc_middle/src/mir/tcx.rs | 2 +- compiler/rustc_middle/src/mir/visit.rs | 4 +- .../src/build/expr/as_place.rs | 2 +- .../rustc_mir_build/src/build/matches/mod.rs | 3 +- .../rustc_mir_build/src/check_unsafety.rs | 4 +- .../src/impls/borrowed_locals.rs | 2 +- .../src/cleanup_post_borrowck.rs | 2 +- .../rustc_mir_transform/src/promote_consts.rs | 2 +- .../rustc_smir/src/rustc_smir/convert/mir.rs | 13 +++- compiler/stable_mir/src/mir/body.rs | 21 ++++-- compiler/stable_mir/src/mir/pretty.rs | 5 +- ...se_edges.full_tested_match.built.after.mir | 2 +- ...e_edges.full_tested_match2.built.after.mir | 2 +- .../match_false_edges.main.built.after.mir | 4 +- ....constant_eq.SimplifyCfg-initial.after.mir | 4 +- ...joint_ranges.SimplifyCfg-initial.after.mir | 2 +- ...fg-initial.after-ElaborateDrops.after.diff | 8 +- ...fg-initial.after-ElaborateDrops.after.diff | 8 +- ...guard.CleanupPostBorrowck.panic-abort.diff | 8 +- ...uard.CleanupPostBorrowck.panic-unwind.diff | 8 +- 32 files changed, 189 insertions(+), 93 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index a38dd286be51b..af5f757107432 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -69,7 +69,8 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> { fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result { let kind = match self.kind { mir::BorrowKind::Shared => "", - mir::BorrowKind::Fake => "fake ", + mir::BorrowKind::Fake(mir::FakeBorrowKind::Deep) => "fake ", + mir::BorrowKind::Fake(mir::FakeBorrowKind::Shallow) => "fake shallow ", mir::BorrowKind::Mut { kind: mir::MutBorrowKind::ClosureCapture } => "uniq ", // FIXME: differentiate `TwoPhaseBorrow` mir::BorrowKind::Mut { diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index ec0d4af599ea5..f6d6b486c8935 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -17,9 +17,9 @@ use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::{ self, AggregateKind, BindingForm, BorrowKind, CallSource, ClearCrossCrate, ConstraintCategory, - FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, MutBorrowKind, Operand, Place, - PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, - VarBindingForm, + FakeBorrowKind, FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, MutBorrowKind, + Operand, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, + TerminatorKind, VarBindingForm, }; use rustc_middle::ty::{ self, suggest_constraining_type_params, PredicateKind, ToPredicate, Ty, TyCtxt, @@ -1486,7 +1486,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let first_borrow_desc; let mut err = match (gen_borrow_kind, issued_borrow.kind) { ( - BorrowKind::Shared, + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow }, ) => { first_borrow_desc = "mutable "; @@ -1504,7 +1504,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } ( BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow }, - BorrowKind::Shared, + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), ) => { first_borrow_desc = "immutable "; let mut err = self.cannot_reborrow_already_borrowed( @@ -1566,7 +1566,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { self.cannot_uniquely_borrow_by_two_closures(span, &desc_place, issued_span, None) } - (BorrowKind::Mut { .. }, BorrowKind::Fake) => { + (BorrowKind::Mut { .. }, BorrowKind::Fake(FakeBorrowKind::Shallow)) => { if let Some(immutable_section_description) = self.classify_immutable_section(issued_borrow.assigned_place) { @@ -1629,7 +1629,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) } - (BorrowKind::Shared, BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture }) => { + ( + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), + BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture }, + ) => { first_borrow_desc = "first "; self.cannot_reborrow_already_uniquely_borrowed( span, @@ -1659,8 +1662,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) } - (BorrowKind::Shared, BorrowKind::Shared | BorrowKind::Fake) - | (BorrowKind::Fake, BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake) => { + ( + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep), + BorrowKind::Shared | BorrowKind::Fake(_), + ) + | ( + BorrowKind::Fake(FakeBorrowKind::Shallow), + BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake(_), + ) => { unreachable!() } }; @@ -3572,7 +3581,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let loan_span = loan_spans.args_or_use(); let descr_place = self.describe_any_place(place.as_ref()); - if loan.kind == BorrowKind::Fake { + if let BorrowKind::Fake(_) = loan.kind { if let Some(section) = self.classify_immutable_section(loan.assigned_place) { let mut err = self.cannot_mutate_in_immutable_section( span, diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index dbea317e7bb26..f88b506eb3e43 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -654,7 +654,7 @@ impl UseSpans<'_> { match kind { Some(kd) => match kd { rustc_middle::mir::BorrowKind::Shared - | rustc_middle::mir::BorrowKind::Fake => { + | rustc_middle::mir::BorrowKind::Fake(_) => { CaptureVarKind::Immut { kind_span: capture_kind_span } } diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 7fbf4c47ec818..e839255c34fa0 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -1056,18 +1056,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Control::Continue } - (Read(_), BorrowKind::Shared | BorrowKind::Fake) - | (Read(ReadKind::Borrow(BorrowKind::Fake)), BorrowKind::Mut { .. }) => { - Control::Continue - } + (Read(_), BorrowKind::Shared | BorrowKind::Fake(_)) + | ( + Read(ReadKind::Borrow(BorrowKind::Fake(FakeBorrowKind::Shallow))), + BorrowKind::Mut { .. }, + ) => Control::Continue, - (Reservation(_), BorrowKind::Fake | BorrowKind::Shared) => { + (Reservation(_), BorrowKind::Fake(_) | BorrowKind::Shared) => { // This used to be a future compatibility warning (to be // disallowed on NLL). See rust-lang/rust#56254 Control::Continue } - (Write(WriteKind::Move), BorrowKind::Fake) => { + (Write(WriteKind::Move), BorrowKind::Fake(FakeBorrowKind::Shallow)) => { // Handled by initialization checks. Control::Continue } @@ -1175,10 +1176,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { match rvalue { &Rvalue::Ref(_ /*rgn*/, bk, place) => { let access_kind = match bk { - BorrowKind::Fake => { + BorrowKind::Fake(FakeBorrowKind::Shallow) => { (Shallow(Some(ArtificialField::FakeBorrow)), Read(ReadKind::Borrow(bk))) } - BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))), + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep) => { + (Deep, Read(ReadKind::Borrow(bk))) + } BorrowKind::Mut { .. } => { let wk = WriteKind::MutableBorrow(bk); if allow_two_phase_borrow(bk) { @@ -1197,7 +1200,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { flow_state, ); - let action = if bk == BorrowKind::Fake { + let action = if bk == BorrowKind::Fake(FakeBorrowKind::Shallow) { InitializationRequiringAction::MatchOn } else { InitializationRequiringAction::Borrow @@ -1556,7 +1559,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // only mutable borrows should be 2-phase assert!(match borrow.kind { - BorrowKind::Shared | BorrowKind::Fake => false, + BorrowKind::Shared | BorrowKind::Fake(_) => false, BorrowKind::Mut { .. } => true, }); @@ -2121,14 +2124,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { | WriteKind::Replace | WriteKind::StorageDeadOrDrop | WriteKind::MutableBorrow(BorrowKind::Shared) - | WriteKind::MutableBorrow(BorrowKind::Fake), + | WriteKind::MutableBorrow(BorrowKind::Fake(_)), ) | Write( WriteKind::Move | WriteKind::Replace | WriteKind::StorageDeadOrDrop | WriteKind::MutableBorrow(BorrowKind::Shared) - | WriteKind::MutableBorrow(BorrowKind::Fake), + | WriteKind::MutableBorrow(BorrowKind::Fake(_)), ) => { if self.is_mutable(place.as_ref(), is_local_mutation_allowed).is_err() && !self.has_buffered_diags() @@ -2152,7 +2155,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { return false; } Read( - ReadKind::Borrow(BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake) + ReadKind::Borrow(BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake(_)) | ReadKind::Copy, ) => { // Access authorized diff --git a/compiler/rustc_borrowck/src/places_conflict.rs b/compiler/rustc_borrowck/src/places_conflict.rs index 7e8dba43b715e..c559f9d574817 100644 --- a/compiler/rustc_borrowck/src/places_conflict.rs +++ b/compiler/rustc_borrowck/src/places_conflict.rs @@ -55,7 +55,7 @@ use crate::Overlap; use crate::{AccessDepth, Deep, Shallow}; use rustc_hir as hir; use rustc_middle::mir::{ - Body, BorrowKind, MutBorrowKind, Place, PlaceElem, PlaceRef, ProjectionElem, + Body, BorrowKind, FakeBorrowKind, MutBorrowKind, Place, PlaceElem, PlaceRef, ProjectionElem, }; use rustc_middle::ty::{self, TyCtxt}; use std::cmp::max; @@ -271,10 +271,10 @@ fn place_components_conflict<'tcx>( // If the second example, where we did, then we still know // that the borrow can access a *part* of our place that // our access cares about, so we still have a conflict. - if borrow_kind == BorrowKind::Fake + if borrow_kind == BorrowKind::Fake(FakeBorrowKind::Shallow) && borrow_place.projection.len() < access_place.projection.len() { - debug!("borrow_conflicts_with_place: fake borrow"); + debug!("borrow_conflicts_with_place: shallow borrow"); false } else { debug!("borrow_conflicts_with_place: full borrow, CONFLICT"); diff --git a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs index 956de1dec9b2b..de469080d14ec 100644 --- a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs +++ b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs @@ -1,6 +1,8 @@ use rustc_data_structures::graph::dominators::Dominators; use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{self, BasicBlock, Body, Location, NonDivergingIntrinsic, Place, Rvalue}; +use rustc_middle::mir::{ + self, BasicBlock, Body, FakeBorrowKind, Location, NonDivergingIntrinsic, Place, Rvalue, +}; use rustc_middle::mir::{BorrowKind, Mutability, Operand}; use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind}; use rustc_middle::mir::{Statement, StatementKind}; @@ -239,10 +241,12 @@ impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> { match rvalue { &Rvalue::Ref(_ /*rgn*/, bk, place) => { let access_kind = match bk { - BorrowKind::Fake => { + BorrowKind::Fake(FakeBorrowKind::Shallow) => { (Shallow(Some(ArtificialField::FakeBorrow)), Read(ReadKind::Borrow(bk))) } - BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))), + BorrowKind::Shared | BorrowKind::Fake(FakeBorrowKind::Deep) => { + (Deep, Read(ReadKind::Borrow(bk))) + } BorrowKind::Mut { .. } => { let wk = WriteKind::MutableBorrow(bk); if allow_two_phase_borrow(bk) { @@ -357,8 +361,11 @@ impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> { // have already taken the reservation } - (Read(_), BorrowKind::Fake | BorrowKind::Shared) - | (Read(ReadKind::Borrow(BorrowKind::Fake)), BorrowKind::Mut { .. }) => { + (Read(_), BorrowKind::Fake(_) | BorrowKind::Shared) + | ( + Read(ReadKind::Borrow(BorrowKind::Fake(FakeBorrowKind::Shallow))), + BorrowKind::Mut { .. }, + ) => { // Reads don't invalidate shared or shallow borrows } @@ -403,7 +410,7 @@ impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> { // only mutable borrows should be 2-phase assert!(match borrow.kind { - BorrowKind::Shared | BorrowKind::Fake => false, + BorrowKind::Shared | BorrowKind::Fake(_) => false, BorrowKind::Mut { .. } => true, }); diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index a506d10c1d089..f9786acfc6c26 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -414,7 +414,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { BorrowKind::Shared => { PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) } - BorrowKind::Fake => { + BorrowKind::Fake(_) => { PlaceContext::NonMutatingUse(NonMutatingUseContext::FakeBorrow) } BorrowKind::Mut { .. } => { @@ -487,7 +487,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { } } - Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Fake, place) + Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Fake(_), place) | Rvalue::AddressOf(Mutability::Not, place) => { let borrowed_place_has_mut_interior = qualifs::in_place::( self.ccx, diff --git a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs index 2c835f6750f75..5ae3ffaaec2f2 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs @@ -105,7 +105,7 @@ where fn ref_allows_mutation(&self, kind: mir::BorrowKind, place: mir::Place<'tcx>) -> bool { match kind { mir::BorrowKind::Mut { .. } => true, - mir::BorrowKind::Shared | mir::BorrowKind::Fake => { + mir::BorrowKind::Shared | mir::BorrowKind::Fake(_) => { self.shared_borrow_allows_mutation(place) } } diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index a499e4b980fc3..69274f962cc3a 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -924,7 +924,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } }, - Rvalue::Ref(_, BorrowKind::Fake, _) => { + Rvalue::Ref(_, BorrowKind::Fake(_), _) => { if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 15bd5c089652c..fead4a16dab28 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -954,7 +954,8 @@ impl<'tcx> Debug for Rvalue<'tcx> { Ref(region, borrow_kind, ref place) => { let kind_str = match borrow_kind { BorrowKind::Shared => "", - BorrowKind::Fake => "fake ", + BorrowKind::Fake(FakeBorrowKind::Deep) => "fake ", + BorrowKind::Fake(FakeBorrowKind::Shallow) => "fake shallow ", BorrowKind::Mut { .. } => "mut ", }; diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs index 235298ffcb8bc..375f1f15a39ed 100644 --- a/compiler/rustc_middle/src/mir/statement.rs +++ b/compiler/rustc_middle/src/mir/statement.rs @@ -451,7 +451,7 @@ impl<'tcx> Rvalue<'tcx> { impl BorrowKind { pub fn mutability(&self) -> Mutability { match *self { - BorrowKind::Shared | BorrowKind::Fake => Mutability::Not, + BorrowKind::Shared | BorrowKind::Fake(_) => Mutability::Not, BorrowKind::Mut { .. } => Mutability::Mut, } } @@ -459,7 +459,7 @@ impl BorrowKind { pub fn allows_two_phase_borrow(&self) -> bool { match *self { BorrowKind::Shared - | BorrowKind::Fake + | BorrowKind::Fake(_) | BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::ClosureCapture } => { false } diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 97c3eb5563861..6292633f65c51 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -165,13 +165,16 @@ pub enum BorrowKind { /// Data must be immutable and is aliasable. Shared, - /// The immediately borrowed place must be immutable, but projections from - /// it don't need to be. For example, a shallow borrow of `a.b` doesn't - /// conflict with a mutable borrow of `a.b.c`. - /// - /// This is used when lowering matches: when matching on a place we want to - /// ensure that place have the same value from the start of the match until - /// an arm is selected. This prevents this code from compiling: + /// An immutable, aliasable borrow that is discarded after borrow-checking. Can behave either + /// like a normal shared borrow or like a special shallow borrow (see [`FakeBorrowKind`]). + /// + /// This is used when lowering index expressions and matches. This is used to prevent code like + /// the following from compiling: + /// ```compile_fail,E0506 + /// let mut x = vec![vec![0, 1]]; + /// let y = vec![]; + /// let _ = x[0][{x = y; 1}]; + /// ``` /// ```compile_fail,E0510 /// let mut x = &Some(0); /// match *x { @@ -180,11 +183,8 @@ pub enum BorrowKind { /// Some(_) => (), /// } /// ``` - /// This can't be a shared borrow because mutably borrowing (*x as Some).0 - /// should not prevent `if let None = x { ... }`, for example, because the - /// mutating `(*x as Some).0` can't affect the discriminant of `x`. /// We can also report errors with this kind of borrow differently. - Fake, + Fake(FakeBorrowKind), /// Data is mutable and not aliasable. Mut { kind: MutBorrowKind }, @@ -240,6 +240,57 @@ pub enum MutBorrowKind { ClosureCapture, } +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, TyEncodable, TyDecodable)] +#[derive(Hash, HashStable)] +pub enum FakeBorrowKind { + /// A shared shallow borrow. The immediately borrowed place must be immutable, but projections + /// from it don't need to be. For example, a shallow borrow of `a.b` doesn't conflict with a + /// mutable borrow of `a.b.c`. + /// + /// This is used when lowering matches: when matching on a place we want to ensure that place + /// have the same value from the start of the match until an arm is selected. This prevents this + /// code from compiling: + /// ```compile_fail,E0510 + /// let mut x = &Some(0); + /// match *x { + /// None => (), + /// Some(_) if { x = &None; false } => (), + /// Some(_) => (), + /// } + /// ``` + /// This can't be a shared borrow because mutably borrowing `(*x as Some).0` should not checking + /// the discriminant or accessing other variants, because the mutating `(*x as Some).0` can't + /// affect the discriminant of `x`. E.g. the following is allowed: + /// ```rust + /// let mut x = Some(0); + /// match x { + /// Some(_) + /// if { + /// if let Some(ref mut y) = x { + /// *y += 1; + /// }; + /// true + /// } => {} + /// _ => {} + /// } + /// ``` + Shallow, + /// A shared (deep) borrow. Data must be immutable and is aliasable. + /// + /// This is used when lowering deref patterns, where shallow borrows wouldn't prevent something + /// like: + // ```compile_fail + // let mut b = Box::new(false); + // match b { + // deref!(true) => {} // not reached because `*b == false` + // _ if { *b = true; false } => {} // not reached because the guard is `false` + // deref!(false) => {} // not reached because the guard changed it + // // UB because we reached the unreachable. + // } + // ``` + Deep, +} + /////////////////////////////////////////////////////////////////////////// // Statements diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index 506003ff7c0cc..4c2bf9abe975d 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -293,7 +293,7 @@ impl BorrowKind { // We have no type corresponding to a shallow borrow, so use // `&` as an approximation. - BorrowKind::Fake => hir::Mutability::Not, + BorrowKind::Fake(_) => hir::Mutability::Not, } } } diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 4f7b2f7cbe48b..0f97bcded3ef1 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -653,7 +653,7 @@ macro_rules! make_mir_visitor { BorrowKind::Shared => PlaceContext::NonMutatingUse( NonMutatingUseContext::SharedBorrow ), - BorrowKind::Fake => PlaceContext::NonMutatingUse( + BorrowKind::Fake(_) => PlaceContext::NonMutatingUse( NonMutatingUseContext::FakeBorrow ), BorrowKind::Mut { .. } => @@ -1279,6 +1279,8 @@ pub enum NonMutatingUseContext { /// Shared borrow. SharedBorrow, /// A fake borrow. + /// FIXME: do we need to distinguish shallow and deep fake borrows? In fact, do we need to + /// distinguish fake and normal deep borrows? FakeBorrow, /// AddressOf for *const pointer. AddressOf, diff --git a/compiler/rustc_mir_build/src/build/expr/as_place.rs b/compiler/rustc_mir_build/src/build/expr/as_place.rs index f12e25db6fcd5..060b328ef48ae 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -685,7 +685,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fake_borrow_temp.into(), Rvalue::Ref( tcx.lifetimes.re_erased, - BorrowKind::Fake, + BorrowKind::Fake(FakeBorrowKind::Shallow), Place { local: base_place.local, projection }, ), ); diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 648d0b52b3214..b41523cb9c25f 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -2106,7 +2106,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { 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, place); + let borrow = + Rvalue::Ref(re_erased, BorrowKind::Fake(FakeBorrowKind::Shallow), place); self.cfg.push_assign(block, scrutinee_source_info, Place::from(temp), borrow); } diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 0c1e1d59c4f3f..227d19c3e43c5 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -513,7 +513,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { visit::walk_expr(&mut visitor, expr); if visitor.found { match borrow_kind { - BorrowKind::Fake | BorrowKind::Shared + BorrowKind::Fake(_) | BorrowKind::Shared if !self.thir[arg].ty.is_freeze(self.tcx, self.param_env) => { self.requires_unsafe(expr.span, BorrowOfLayoutConstrainedField) @@ -521,7 +521,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { BorrowKind::Mut { .. } => { self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField) } - BorrowKind::Fake | BorrowKind::Shared => {} + BorrowKind::Fake(_) | BorrowKind::Shared => {} } } } diff --git a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs index 693994b5da76f..bdc70de58e84b 100644 --- a/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs @@ -102,7 +102,7 @@ where } Rvalue::Cast(..) - | Rvalue::Ref(_, BorrowKind::Fake, _) + | Rvalue::Ref(_, BorrowKind::Fake(_), _) | Rvalue::ShallowInitBox(..) | Rvalue::Use(..) | Rvalue::ThreadLocalRef(..) diff --git a/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs b/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs index da82f8de78147..48a6a83e14609 100644 --- a/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs +++ b/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs @@ -29,7 +29,7 @@ impl<'tcx> MirPass<'tcx> for CleanupPostBorrowck { for statement in basic_block.statements.iter_mut() { match statement.kind { StatementKind::AscribeUserType(..) - | StatementKind::Assign(box (_, Rvalue::Ref(_, BorrowKind::Fake, _))) + | StatementKind::Assign(box (_, Rvalue::Ref(_, BorrowKind::Fake(_), _))) | StatementKind::Coverage( // These kinds of coverage statements are markers inserted during // MIR building, and are not needed after InstrumentCoverage. diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index 1f4af0ec63dd9..c14d4ceb09106 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -384,7 +384,7 @@ impl<'tcx> Validator<'_, 'tcx> { match kind { // Reject these borrow types just to be safe. // FIXME(RalfJung): could we allow them? Should we? No point in it until we have a usecase. - BorrowKind::Fake | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => { + BorrowKind::Fake(_) | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => { return Err(Unpromotable); } diff --git a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs index c9f6661259022..a701dfe7046d0 100644 --- a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs +++ b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs @@ -229,7 +229,7 @@ impl<'tcx> Stable<'tcx> for mir::BorrowKind { use rustc_middle::mir::BorrowKind::*; match *self { Shared => stable_mir::mir::BorrowKind::Shared, - Fake => stable_mir::mir::BorrowKind::Fake, + Fake(kind) => stable_mir::mir::BorrowKind::Fake(kind.stable(tables)), Mut { kind } => stable_mir::mir::BorrowKind::Mut { kind: kind.stable(tables) }, } } @@ -247,6 +247,17 @@ impl<'tcx> Stable<'tcx> for mir::MutBorrowKind { } } +impl<'tcx> Stable<'tcx> for mir::FakeBorrowKind { + type T = stable_mir::mir::FakeBorrowKind; + fn stable(&self, _: &mut Tables<'_>) -> Self::T { + use rustc_middle::mir::FakeBorrowKind::*; + match *self { + Deep => stable_mir::mir::FakeBorrowKind::Deep, + Shallow => stable_mir::mir::FakeBorrowKind::Shallow, + } + } +} + impl<'tcx> Stable<'tcx> for mir::NullOp<'tcx> { type T = stable_mir::mir::NullOp; fn stable(&self, tables: &mut Tables<'_>) -> Self::T { diff --git a/compiler/stable_mir/src/mir/body.rs b/compiler/stable_mir/src/mir/body.rs index 1ad05633d62dc..e928a6f3dcdba 100644 --- a/compiler/stable_mir/src/mir/body.rs +++ b/compiler/stable_mir/src/mir/body.rs @@ -865,11 +865,9 @@ pub enum BorrowKind { /// Data must be immutable and is aliasable. Shared, - /// The immediately borrowed place must be immutable, but projections from - /// it don't need to be. This is used to prevent match guards from replacing - /// the scrutinee. For example, a fake borrow of `a.b` doesn't - /// conflict with a mutable borrow of `a.b.c`. - Fake, + /// An immutable, aliasable borrow that is discarded after borrow-checking. Can behave either + /// like a normal shared borrow or like a special shallow borrow (see [`FakeBorrowKind`]). + Fake(FakeBorrowKind), /// Data is mutable and not aliasable. Mut { @@ -884,7 +882,7 @@ impl BorrowKind { BorrowKind::Mut { .. } => Mutability::Mut, BorrowKind::Shared => Mutability::Not, // FIXME: There's no type corresponding to a shallow borrow, so use `&` as an approximation. - BorrowKind::Fake => Mutability::Not, + BorrowKind::Fake(_) => Mutability::Not, } } } @@ -896,6 +894,17 @@ pub enum MutBorrowKind { ClosureCapture, } +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum FakeBorrowKind { + /// A shared (deep) borrow. Data must be immutable and is aliasable. + Deep, + /// The immediately borrowed place must be immutable, but projections from + /// it don't need to be. This is used to prevent match guards from replacing + /// the scrutinee. For example, a fake borrow of `a.b` doesn't + /// conflict with a mutable borrow of `a.b.c`. + Shallow, +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum Mutability { Not, diff --git a/compiler/stable_mir/src/mir/pretty.rs b/compiler/stable_mir/src/mir/pretty.rs index 4ac4833add715..bbca3965852a6 100644 --- a/compiler/stable_mir/src/mir/pretty.rs +++ b/compiler/stable_mir/src/mir/pretty.rs @@ -8,7 +8,7 @@ use std::{fmt, io, iter}; use super::{AssertMessage, BinOp, TerminatorKind}; -use super::BorrowKind; +use super::{BorrowKind, FakeBorrowKind}; impl Display for Ty { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -352,7 +352,8 @@ fn pretty_rvalue(writer: &mut W, rval: &Rvalue) -> io::Result<()> { Rvalue::Ref(_, borrowkind, place) => { let kind = match borrowkind { BorrowKind::Shared => "&", - BorrowKind::Fake => "&fake ", + BorrowKind::Fake(FakeBorrowKind::Deep) => "&fake ", + BorrowKind::Fake(FakeBorrowKind::Shallow) => "&fake shallow ", BorrowKind::Mut { .. } => "&mut ", }; write!(writer, "{kind}{:?}", place) diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir index 0d381ce70f175..bade0fa4b45c8 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match.built.after.mir @@ -60,7 +60,7 @@ fn full_tested_match() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } diff --git a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir index d397de66fc470..0d78bb8b23587 100644 --- a/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.full_tested_match2.built.after.mir @@ -66,7 +66,7 @@ fn full_tested_match2() -> () { bb7: { StorageLive(_6); _6 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_7); _7 = guard() -> [return: bb8, unwind: bb16]; } diff --git a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir index 63920cec885ad..ebb75ae141a33 100644 --- a/tests/mir-opt/building/match/match_false_edges.main.built.after.mir +++ b/tests/mir-opt/building/match/match_false_edges.main.built.after.mir @@ -87,7 +87,7 @@ fn main() -> () { bb10: { StorageLive(_7); _7 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_8); _8 = guard() -> [return: bb11, unwind: bb24]; } @@ -129,7 +129,7 @@ fn main() -> () { bb16: { StorageLive(_11); _11 = &((_2 as Some).0: i32); - _3 = &fake _2; + _3 = &fake shallow _2; StorageLive(_12); StorageLive(_13); _13 = (*_11); diff --git a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir index 21ddd39137fb2..060cd6132e3a0 100644 --- a/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir @@ -72,8 +72,8 @@ fn constant_eq(_1: &str, _2: bool) -> u32 { } bb12: { - _6 = &fake (_3.0: &str); - _7 = &fake (_3.1: bool); + _6 = &fake shallow (_3.0: &str); + _7 = &fake shallow (_3.1: bool); StorageLive(_10); _10 = const true; switchInt(move _10) -> [0: bb14, otherwise: bb13]; diff --git a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir index 6068cef8fbcb0..07daa3eddfa2c 100644 --- a/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir @@ -54,7 +54,7 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 { } bb9: { - _3 = &fake _1; + _3 = &fake shallow _1; StorageLive(_8); _8 = _2; switchInt(move _8) -> [0: bb11, otherwise: bb10]; diff --git a/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff b/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff index ba333ba1a5866..209f0d09c2943 100644 --- a/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff +++ b/tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff @@ -80,8 +80,8 @@ _6 = &(_2.1: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_9); StorageLive(_10); _10 = _1; @@ -137,8 +137,8 @@ _6 = &(_2.0: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_12); StorageLive(_13); _13 = _1; diff --git a/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff b/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff index ba333ba1a5866..209f0d09c2943 100644 --- a/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff +++ b/tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff @@ -80,8 +80,8 @@ _6 = &(_2.1: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_9); StorageLive(_10); _10 = _1; @@ -137,8 +137,8 @@ _6 = &(_2.0: bool); StorageLive(_8); _8 = &(_2.2: std::string::String); -- _3 = &fake (_2.0: bool); -- _4 = &fake (_2.1: bool); +- _3 = &fake shallow (_2.0: bool); +- _4 = &fake shallow (_2.1: bool); StorageLive(_12); StorageLive(_13); _13 = _1; diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff index 8feb59ec8747b..d76d65a18a7b4 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-abort.diff @@ -33,10 +33,10 @@ } bb4: { -- _3 = &fake _1; -- _4 = &fake (*(*((_1 as Some).0: &&i32))); -- _5 = &fake (*((_1 as Some).0: &&i32)); -- _6 = &fake ((_1 as Some).0: &&i32); +- _3 = &fake shallow _1; +- _4 = &fake shallow (*(*((_1 as Some).0: &&i32))); +- _5 = &fake shallow (*((_1 as Some).0: &&i32)); +- _6 = &fake shallow ((_1 as Some).0: &&i32); + nop; + nop; + nop; diff --git a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff index 8feb59ec8747b..d76d65a18a7b4 100644 --- a/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff +++ b/tests/mir-opt/remove_fake_borrows.match_guard.CleanupPostBorrowck.panic-unwind.diff @@ -33,10 +33,10 @@ } bb4: { -- _3 = &fake _1; -- _4 = &fake (*(*((_1 as Some).0: &&i32))); -- _5 = &fake (*((_1 as Some).0: &&i32)); -- _6 = &fake ((_1 as Some).0: &&i32); +- _3 = &fake shallow _1; +- _4 = &fake shallow (*(*((_1 as Some).0: &&i32))); +- _5 = &fake shallow (*((_1 as Some).0: &&i32)); +- _6 = &fake shallow ((_1 as Some).0: &&i32); + nop; + nop; + nop; From 436c61266c7e9c4d6b7a57a8a6170700eda68fd0 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 6 Apr 2024 00:29:45 +0200 Subject: [PATCH 08/10] 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 b41523cb9c25f..2ab297a47ae02 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 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { 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 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { 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 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &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 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &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 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { 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 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { 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 967b0c44588fa..2f9390c22a89d 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 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, '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 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, '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 0000000000000..35fa9cbf7d859 --- /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 0000000000000..6a591e6416c5d --- /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`. From 217a4dff7da0e4be2c22fd8f822cb543745c6727 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 8 Apr 2024 20:05:23 +0200 Subject: [PATCH 09/10] Test or-patterns inside deref patterns --- tests/ui/pattern/deref-patterns/bindings.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index 4f72058af8fe5..5881e4166a46f 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -37,6 +37,17 @@ fn ref_mut(val: u32) -> u32 { *x } +#[rustfmt::skip] +fn or_and_guard(tuple: (u32, u32)) -> u32 { + let mut sum = 0; + let b = Box::new(tuple); + match b { + deref!((x, _) | (_, x)) if { sum += x; false } => {}, + _ => {}, + } + sum +} + fn main() { assert_eq!(simple_vec(vec![1]), 1); assert_eq!(simple_vec(vec![1, 2]), 202); @@ -48,5 +59,6 @@ fn main() { assert_eq!(nested_vec(vec![vec![1, 2, 3]]), 6); assert_eq!(nested_vec(vec![vec![], vec![1, 2, 3]]), 1); - assert_eq!(ref_mut(42), 42) + assert_eq!(ref_mut(42), 42); + assert_eq!(or_and_guard((10, 32)), 42); } From 726fb55ae29a778a698e1c880aa0ae46920ece88 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 20 Apr 2024 16:07:27 +0200 Subject: [PATCH 10/10] Fix documentation of `BorrowKind::Fake` --- compiler/rustc_middle/src/mir/syntax.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 6292633f65c51..e07b6c6bd81f9 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -170,9 +170,9 @@ pub enum BorrowKind { /// /// This is used when lowering index expressions and matches. This is used to prevent code like /// the following from compiling: - /// ```compile_fail,E0506 - /// let mut x = vec![vec![0, 1]]; - /// let y = vec![]; + /// ```compile_fail,E0510 + /// let mut x: &[_] = &[[0, 1]]; + /// let y: &[_] = &[]; /// let _ = x[0][{x = y; 1}]; /// ``` /// ```compile_fail,E0510