From a52cc0a8c967253e0f61c7f77c820ab6e1f2e73e Mon Sep 17 00:00:00 2001 From: Ziru Niu Date: Wed, 31 May 2023 19:13:13 +0800 Subject: [PATCH] address most easy comments --- compiler/rustc_borrowck/src/borrow_set.rs | 5 ++++- .../rustc_borrowck/src/diagnostics/mod.rs | 7 +++---- compiler/rustc_borrowck/src/lib.rs | 15 +++++++------- .../src/transform/check_consts/check.rs | 8 ++------ .../src/transform/promote_consts.rs | 4 +++- compiler/rustc_middle/src/mir/mod.rs | 14 ++++--------- compiler/rustc_middle/src/ty/closure.rs | 2 ++ .../rustc_mir_build/src/check_unsafety.rs | 20 +++++++------------ 8 files changed, 33 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index cf17b16b63a..0b44beeb004 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -73,7 +73,10 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> { mir::BorrowKind::Shared => "", mir::BorrowKind::Shallow => "shallow ", mir::BorrowKind::Mut { kind: mir::MutBorrowKind::ClosureCapture } => "uniq ", - mir::BorrowKind::Mut { .. } => "mut ", + // FIXME: differentiate `TwoPhaseBorrow` + mir::BorrowKind::Mut { + kind: mir::MutBorrowKind::Default | mir::MutBorrowKind::TwoPhaseBorrow, + } => "mut ", }; write!(w, "&{:?} {}{:?}", self.region, kind, self.borrowed_place) } diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 931ee8767d9..2f8c970f806 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -628,10 +628,9 @@ impl UseSpans<'_> { err.subdiagnostic(match kind { Some(kd) => match kd { rustc_middle::mir::BorrowKind::Shared - | rustc_middle::mir::BorrowKind::Shallow - | rustc_middle::mir::BorrowKind::Mut { - kind: rustc_middle::mir::MutBorrowKind::ClosureCapture, - } => CaptureVarKind::Immut { kind_span: capture_kind_span }, + | rustc_middle::mir::BorrowKind::Shallow => { + CaptureVarKind::Immut { kind_span: capture_kind_span } + } rustc_middle::mir::BorrowKind::Mut { .. } => { CaptureVarKind::Mut { kind_span: capture_kind_span } diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index d27dc542ad5..be3a3b77797 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -1958,14 +1958,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let the_place_err; match kind { - Reservation(WriteKind::MutableBorrow(borrow_kind @ BorrowKind::Mut { .. })) - | Write(WriteKind::MutableBorrow(borrow_kind @ BorrowKind::Mut { .. })) => { - let is_local_mutation_allowed = match borrow_kind { - BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => { - LocalMutationIsAllowed::Yes + Reservation(WriteKind::MutableBorrow(BorrowKind::Mut { kind: mut_borrow_kind })) + | Write(WriteKind::MutableBorrow(BorrowKind::Mut { kind: mut_borrow_kind })) => { + let is_local_mutation_allowed = match mut_borrow_kind { + // `ClosureCapture` is used for mutable variable with a immutable binding. + // This is only behaviour difference between `ClosureCapture` and mutable borrows. + MutBorrowKind::ClosureCapture => LocalMutationIsAllowed::Yes, + MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow => { + is_local_mutation_allowed } - BorrowKind::Mut { .. } => is_local_mutation_allowed, - BorrowKind::Shared | BorrowKind::Shallow => unreachable!(), }; match self.is_mutable(place.as_ref(), is_local_mutation_allowed) { Ok(root_place) => { 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 ee85d12a273..33c79ad7e55 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -456,7 +456,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { } } - Rvalue::Ref(_, kind @ BorrowKind::Mut { .. }, place) => { + Rvalue::Ref(_, BorrowKind::Mut { .. }, place) => { let ty = place.ty(self.body, self.tcx).ty; let is_allowed = match ty.kind() { // Inside a `static mut`, `&mut [...]` is allowed. @@ -477,11 +477,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { }; if !is_allowed { - if let BorrowKind::Mut { .. } = kind { - self.check_mut_borrow(place.local, hir::BorrowKind::Ref) - } else { - self.check_op(ops::CellBorrow); - } + self.check_mut_borrow(place.local, hir::BorrowKind::Ref) } } diff --git a/compiler/rustc_const_eval/src/transform/promote_consts.rs b/compiler/rustc_const_eval/src/transform/promote_consts.rs index c1abc4b5e99..dd80f745c2f 100644 --- a/compiler/rustc_const_eval/src/transform/promote_consts.rs +++ b/compiler/rustc_const_eval/src/transform/promote_consts.rs @@ -465,7 +465,9 @@ impl<'tcx> Validator<'_, 'tcx> { } } - BorrowKind::Mut { .. } => { + // FIXME: consider changing this to only promote &mut [] for default borrows, + // also forbidding two phase borrows + BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow } => { let ty = place.ty(self.body, self.tcx).ty; // In theory, any zero-sized value could be borrowed diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 7b61c375707..669c609d995 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2041,19 +2041,13 @@ impl BorrowKind { } pub fn allows_two_phase_borrow(&self) -> bool { - match *self { - BorrowKind::Shared | BorrowKind::Shallow => false, - BorrowKind::Mut { kind } => kind == MutBorrowKind::TwoPhaseBorrow, - } - } - - // FIXME: won't be used after diagnostic migration - pub fn describe_mutability(&self) -> &str { match *self { BorrowKind::Shared | BorrowKind::Shallow - | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => "immutable", - BorrowKind::Mut { .. } => "mutable", + | BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::ClosureCapture } => { + false + } + BorrowKind::Mut { kind: MutBorrowKind::TwoPhaseBorrow } => true, } } } diff --git a/compiler/rustc_middle/src/ty/closure.rs b/compiler/rustc_middle/src/ty/closure.rs index be7b2b7ec67..bc927374549 100644 --- a/compiler/rustc_middle/src/ty/closure.rs +++ b/compiler/rustc_middle/src/ty/closure.rs @@ -427,6 +427,8 @@ pub enum BorrowKind { /// 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. + /// + /// FIXME: Rename this to indicate the borrow is actually not immutable. UniqueImmBorrow, /// Data is mutable and not aliasable. diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index f7ef8485582..4f3a574031d 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -3,7 +3,7 @@ use crate::errors::*; use rustc_middle::thir::visit::{self, Visitor}; use rustc_hir as hir; -use rustc_middle::mir::{BorrowKind, MutBorrowKind}; +use rustc_middle::mir::BorrowKind; use rustc_middle::thir::*; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt}; @@ -254,9 +254,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { ); }; match borrow_kind { - BorrowKind::Shallow - | BorrowKind::Shared - | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => { + BorrowKind::Shallow | BorrowKind::Shared => { if !ty.is_freeze(self.tcx, self.param_env) { self.requires_unsafe(pat.span, BorrowOfLayoutConstrainedField); } @@ -442,19 +440,15 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { visit::walk_expr(&mut visitor, expr); if visitor.found { match borrow_kind { - BorrowKind::Shallow - | BorrowKind::Shared - | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } + BorrowKind::Shallow | BorrowKind::Shared if !self.thir[arg].ty.is_freeze(self.tcx, self.param_env) => { self.requires_unsafe(expr.span, BorrowOfLayoutConstrainedField) } - BorrowKind::Mut { - kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow, - } => self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField), - BorrowKind::Shallow - | BorrowKind::Shared - | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => {} + BorrowKind::Mut { .. } => { + self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField) + } + BorrowKind::Shallow | BorrowKind::Shared => {} } } }