From a336bdc1428c88ef85c1cc030a931a36930f7f63 Mon Sep 17 00:00:00 2001 From: Shoyu Vanilla Date: Mon, 26 Feb 2024 01:57:03 +0900 Subject: [PATCH] Merge `BorrowKind::Unique` into `BorrowKind::Mut` --- crates/hir-ty/src/infer/closure.rs | 34 ++++++----- crates/hir-ty/src/mir.rs | 61 +++++-------------- crates/hir-ty/src/mir/borrowck.rs | 12 +++- crates/hir-ty/src/mir/lower/as_place.rs | 4 +- .../hir-ty/src/mir/lower/pattern_matching.rs | 17 +++--- crates/hir-ty/src/mir/pretty.rs | 9 ++- crates/hir/src/lib.rs | 14 ++--- 7 files changed, 68 insertions(+), 83 deletions(-) diff --git a/crates/hir-ty/src/infer/closure.rs b/crates/hir-ty/src/infer/closure.rs index 22a70f951ea..000248c2ce2 100644 --- a/crates/hir-ty/src/infer/closure.rs +++ b/crates/hir-ty/src/infer/closure.rs @@ -23,7 +23,7 @@ use crate::{ db::{HirDatabase, InternedClosure}, from_placeholder_idx, make_binders, - mir::{BorrowKind, MirSpan, ProjectionElem}, + mir::{BorrowKind, MirSpan, MutBorrowKind, ProjectionElem}, static_lifetime, to_chalk_trait_id, traits::FnTrait, utils::{self, generics, Generics}, @@ -142,9 +142,13 @@ fn capture_kind_of_truncated_place( mut current_capture: CaptureKind, len: usize, ) -> CaptureKind { - if let CaptureKind::ByRef(BorrowKind::Mut { .. }) = current_capture { + if let CaptureKind::ByRef(BorrowKind::Mut { + kind: MutBorrowKind::Default | MutBorrowKind::TwoPhasedBorrow, + }) = current_capture + { if self.projections[len..].iter().any(|it| *it == ProjectionElem::Deref) { - current_capture = CaptureKind::ByRef(BorrowKind::Unique); + current_capture = + CaptureKind::ByRef(BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture }); } } current_capture @@ -377,7 +381,7 @@ fn mutate_expr(&mut self, expr: ExprId) { if let Some(place) = self.place_of_expr(expr) { self.add_capture( place, - CaptureKind::ByRef(BorrowKind::Mut { allow_two_phase_borrow: false }), + CaptureKind::ByRef(BorrowKind::Mut { kind: MutBorrowKind::Default }), expr.into(), ); } @@ -426,9 +430,7 @@ fn walk_expr_with_adjust(&mut self, tgt_expr: ExprId, adjustment: &[Adjustment]) fn ref_capture_with_adjusts(&mut self, m: Mutability, tgt_expr: ExprId, rest: &[Adjustment]) { let capture_kind = match m { - Mutability::Mut => { - CaptureKind::ByRef(BorrowKind::Mut { allow_two_phase_borrow: false }) - } + Mutability::Mut => CaptureKind::ByRef(BorrowKind::Mut { kind: MutBorrowKind::Default }), Mutability::Not => CaptureKind::ByRef(BorrowKind::Shared), }; if let Some(place) = self.place_of_expr_without_adjust(tgt_expr) { @@ -648,7 +650,7 @@ fn walk_pat(&mut self, result: &mut Option, pat: PatId) { self.walk_pat_inner( pat, &mut update_result, - BorrowKind::Mut { allow_two_phase_borrow: false }, + BorrowKind::Mut { kind: MutBorrowKind::Default }, ); } @@ -699,7 +701,7 @@ fn walk_pat_inner( }, } if self.result.pat_adjustments.get(&p).map_or(false, |it| !it.is_empty()) { - for_mut = BorrowKind::Unique; + for_mut = BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture }; } self.body.walk_pats_shallow(p, |p| self.walk_pat_inner(p, update_result, for_mut)); } @@ -880,7 +882,7 @@ fn consume_with_pat(&mut self, mut place: HirPlace, pat: PatId) { } BindingMode::Ref(Mutability::Not) => BorrowKind::Shared, BindingMode::Ref(Mutability::Mut) => { - BorrowKind::Mut { allow_two_phase_borrow: false } + BorrowKind::Mut { kind: MutBorrowKind::Default } } }; self.add_capture(place, CaptureKind::ByRef(capture_kind), pat.into()); @@ -930,9 +932,7 @@ fn closure_kind(&self) -> FnTrait { r = cmp::min( r, match &it.kind { - CaptureKind::ByRef(BorrowKind::Unique | BorrowKind::Mut { .. }) => { - FnTrait::FnMut - } + CaptureKind::ByRef(BorrowKind::Mut { .. }) => FnTrait::FnMut, CaptureKind::ByRef(BorrowKind::Shallow | BorrowKind::Shared) => FnTrait::Fn, CaptureKind::ByValue => FnTrait::FnOnce, }, @@ -949,8 +949,12 @@ fn analyze_closure(&mut self, closure: ClosureId) -> FnTrait { }; self.consume_expr(*body); for item in &self.current_captures { - if matches!(item.kind, CaptureKind::ByRef(BorrowKind::Mut { .. })) - && !item.place.projections.contains(&ProjectionElem::Deref) + if matches!( + item.kind, + CaptureKind::ByRef(BorrowKind::Mut { + kind: MutBorrowKind::Default | MutBorrowKind::TwoPhasedBorrow + }) + ) && !item.place.projections.contains(&ProjectionElem::Deref) { // FIXME: remove the `mutated_bindings_in_closure` completely and add proper fake reads in // MIR. I didn't do that due duplicate diagnostics. diff --git a/crates/hir-ty/src/mir.rs b/crates/hir-ty/src/mir.rs index 494f1850b88..cfaef2a392c 100644 --- a/crates/hir-ty/src/mir.rs +++ b/crates/hir-ty/src/mir.rs @@ -659,66 +659,33 @@ pub enum BorrowKind { /// We can also report errors with this kind of borrow differently. Shallow, - /// Data must be immutable but not aliasable. This kind of borrow - /// cannot currently be expressed by the user and is used only in - /// implicit closure bindings. It is needed when the closure is - /// borrowing or mutating a mutable referent, e.g.: - /// ``` - /// let mut z = 3; - /// let x: &mut isize = &mut z; - /// let y = || *x += 5; - /// ``` - /// If we were to try to translate this closure into a more explicit - /// form, we'd encounter an error with the code as written: - /// ```compile_fail,E0594 - /// struct Env<'a> { x: &'a &'a mut isize } - /// let mut z = 3; - /// let x: &mut isize = &mut z; - /// let y = (&mut Env { x: &x }, fn_ptr); // Closure is pair of env and fn - /// fn fn_ptr(env: &mut Env) { **env.x += 5; } - /// ``` - /// This is then illegal because you cannot mutate an `&mut` found - /// in an aliasable location. To solve, you'd have to translate with - /// an `&mut` borrow: - /// ```compile_fail,E0596 - /// struct Env<'a> { x: &'a mut &'a mut isize } - /// let mut z = 3; - /// let x: &mut isize = &mut z; - /// let y = (&mut Env { x: &mut x }, fn_ptr); // changed from &x to &mut x - /// fn fn_ptr(env: &mut Env) { **env.x += 5; } - /// ``` - /// Now the assignment to `**env.x` is legal, but creating a - /// mutable pointer to `x` is not because `x` is not mutable. We - /// could fix this by declaring `x` as `let mut x`. This is ok in - /// user code, if awkward, but extra weird for closures, since the - /// borrow is hidden. - /// - /// So we introduce a "unique imm" borrow -- the referent is - /// immutable, but not aliasable. This solves the problem. For - /// simplicity, we don't give users the way to express this - /// borrow, it's just used when translating closures. - Unique, - /// Data is mutable and not aliasable. - Mut { - /// `true` if this borrow arose from method-call auto-ref - /// (i.e., `adjustment::Adjust::Borrow`). - allow_two_phase_borrow: bool, - }, + Mut { kind: MutBorrowKind }, +} + +#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord)] +pub enum MutBorrowKind { + Default, + /// This borrow arose from method-call auto-ref + /// (i.e., adjustment::Adjust::Borrow). + TwoPhasedBorrow, + /// Data must be immutable but not aliasable. This kind of borrow cannot currently + /// be expressed by the user and is used only in implicit closure bindings. + ClosureCapture, } impl BorrowKind { fn from_hir(m: hir_def::type_ref::Mutability) -> Self { match m { hir_def::type_ref::Mutability::Shared => BorrowKind::Shared, - hir_def::type_ref::Mutability::Mut => BorrowKind::Mut { allow_two_phase_borrow: false }, + hir_def::type_ref::Mutability::Mut => BorrowKind::Mut { kind: MutBorrowKind::Default }, } } fn from_chalk(m: Mutability) -> Self { match m { Mutability::Not => BorrowKind::Shared, - Mutability::Mut => BorrowKind::Mut { allow_two_phase_borrow: false }, + Mutability::Mut => BorrowKind::Mut { kind: MutBorrowKind::Default }, } } } diff --git a/crates/hir-ty/src/mir/borrowck.rs b/crates/hir-ty/src/mir/borrowck.rs index 63fa87ad662..8b6936f8bc0 100644 --- a/crates/hir-ty/src/mir/borrowck.rs +++ b/crates/hir-ty/src/mir/borrowck.rs @@ -19,8 +19,8 @@ }; use super::{ - BasicBlockId, BorrowKind, LocalId, MirBody, MirLowerError, MirSpan, Place, ProjectionElem, - Rvalue, StatementKind, TerminatorKind, + BasicBlockId, BorrowKind, LocalId, MirBody, MirLowerError, MirSpan, MutBorrowKind, Place, + ProjectionElem, Rvalue, StatementKind, TerminatorKind, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -540,7 +540,13 @@ fn mutability_of_locals( } Rvalue::ShallowInitBox(_, _) | Rvalue::ShallowInitBoxWithAlloc(_) => (), } - if let Rvalue::Ref(BorrowKind::Mut { .. }, p) = value { + if let Rvalue::Ref( + BorrowKind::Mut { + kind: MutBorrowKind::Default | MutBorrowKind::TwoPhasedBorrow, + }, + p, + ) = value + { if place_case(db, body, p) != ProjectionCase::Indirect { push_mut_span(p.local, statement.span, &mut result); } diff --git a/crates/hir-ty/src/mir/lower/as_place.rs b/crates/hir-ty/src/mir/lower/as_place.rs index afe33607d46..be81915bb40 100644 --- a/crates/hir-ty/src/mir/lower/as_place.rs +++ b/crates/hir-ty/src/mir/lower/as_place.rs @@ -1,5 +1,7 @@ //! MIR lowering for places +use crate::mir::MutBorrowKind; + use super::*; use hir_def::FunctionId; use hir_expand::name; @@ -328,7 +330,7 @@ fn lower_overloaded_deref( Mutability::Mut, LangItem::DerefMut, name![deref_mut], - BorrowKind::Mut { allow_two_phase_borrow: false }, + BorrowKind::Mut { kind: MutBorrowKind::Default }, ) }; let ty_ref = TyKind::Ref(chalk_mut, static_lifetime(), source_ty.clone()).intern(Interner); diff --git a/crates/hir-ty/src/mir/lower/pattern_matching.rs b/crates/hir-ty/src/mir/lower/pattern_matching.rs index 85c8d1685b8..90cbd13a6c6 100644 --- a/crates/hir-ty/src/mir/lower/pattern_matching.rs +++ b/crates/hir-ty/src/mir/lower/pattern_matching.rs @@ -3,12 +3,15 @@ use hir_def::{hir::LiteralOrConst, resolver::HasResolver, AssocItemId}; use crate::{ - mir::lower::{ - BasicBlockId, BinOp, BindingId, BorrowKind, Either, Expr, FieldId, Idx, Interner, - MemoryMap, MirLowerCtx, MirLowerError, MirSpan, Mutability, Operand, Pat, PatId, Place, - PlaceElem, ProjectionElem, RecordFieldPat, ResolveValueResult, Result, Rvalue, - Substitution, SwitchTargets, TerminatorKind, TupleFieldId, TupleId, TyBuilder, TyKind, - ValueNs, VariantData, VariantId, + mir::{ + lower::{ + BasicBlockId, BinOp, BindingId, BorrowKind, Either, Expr, FieldId, Idx, Interner, + MemoryMap, MirLowerCtx, MirLowerError, MirSpan, Mutability, Operand, Pat, PatId, Place, + PlaceElem, ProjectionElem, RecordFieldPat, ResolveValueResult, Result, Rvalue, + Substitution, SwitchTargets, TerminatorKind, TupleFieldId, TupleId, TyBuilder, TyKind, + ValueNs, VariantData, VariantId, + }, + MutBorrowKind, }, BindingMode, }; @@ -450,7 +453,7 @@ fn pattern_match_binding( BindingMode::Move => Operand::Copy(cond_place).into(), BindingMode::Ref(Mutability::Not) => Rvalue::Ref(BorrowKind::Shared, cond_place), BindingMode::Ref(Mutability::Mut) => { - Rvalue::Ref(BorrowKind::Mut { allow_two_phase_borrow: false }, cond_place) + Rvalue::Ref(BorrowKind::Mut { kind: MutBorrowKind::Default }, cond_place) } }, span, diff --git a/crates/hir-ty/src/mir/pretty.rs b/crates/hir-ty/src/mir/pretty.rs index 23fc2713554..0c641d7c6c2 100644 --- a/crates/hir-ty/src/mir/pretty.rs +++ b/crates/hir-ty/src/mir/pretty.rs @@ -18,7 +18,8 @@ }; use super::{ - AggregateKind, BasicBlockId, BorrowKind, LocalId, MirBody, Operand, Place, Rvalue, UnOp, + AggregateKind, BasicBlockId, BorrowKind, LocalId, MirBody, MutBorrowKind, Operand, Place, + Rvalue, UnOp, }; macro_rules! w { @@ -366,8 +367,10 @@ fn rvalue(&mut self, r: &Rvalue) { match r { BorrowKind::Shared => w!(self, "&"), BorrowKind::Shallow => w!(self, "&shallow "), - BorrowKind::Unique => w!(self, "&uniq "), - BorrowKind::Mut { .. } => w!(self, "&mut "), + BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => w!(self, "&uniq "), + BorrowKind::Mut { + kind: MutBorrowKind::Default | MutBorrowKind::TwoPhasedBorrow, + } => w!(self, "&mut "), } self.place(p); } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 2d8811cf5eb..a0237e3b90b 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -68,7 +68,7 @@ known_const_to_ast, layout::{Layout as TyLayout, RustcEnumVariantIdx, RustcFieldIdx, TagEncoding}, method_resolution::{self, TyFingerprint}, - mir::interpret_mir, + mir::{interpret_mir, MutBorrowKind}, primitive::UintTy, traits::FnTrait, AliasTy, CallableDefId, CallableSig, Canonical, CanonicalVarKinds, Cast, ClosureId, GenericArg, @@ -3754,12 +3754,12 @@ pub fn kind(&self) -> CaptureKind { hir_ty::CaptureKind::ByRef( hir_ty::mir::BorrowKind::Shallow | hir_ty::mir::BorrowKind::Shared, ) => CaptureKind::SharedRef, - hir_ty::CaptureKind::ByRef(hir_ty::mir::BorrowKind::Unique) => { - CaptureKind::UniqueSharedRef - } - hir_ty::CaptureKind::ByRef(hir_ty::mir::BorrowKind::Mut { .. }) => { - CaptureKind::MutableRef - } + hir_ty::CaptureKind::ByRef(hir_ty::mir::BorrowKind::Mut { + kind: MutBorrowKind::ClosureCapture, + }) => CaptureKind::UniqueSharedRef, + hir_ty::CaptureKind::ByRef(hir_ty::mir::BorrowKind::Mut { + kind: MutBorrowKind::Default | MutBorrowKind::TwoPhasedBorrow, + }) => CaptureKind::MutableRef, hir_ty::CaptureKind::ByValue => CaptureKind::Move, } }