Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge BorrowKind::Unique into BorrowKind::Mut #16669

Merged
merged 1 commit into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions crates/hir-ty/src/infer/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use stdx::never;
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},
Expand Down Expand Up @@ -142,9 +142,13 @@ impl HirPlace {
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
Expand Down Expand Up @@ -377,7 +381,7 @@ impl InferenceContext<'_> {
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(),
);
}
Expand Down Expand Up @@ -426,9 +430,7 @@ impl InferenceContext<'_> {

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) {
Expand Down Expand Up @@ -648,7 +650,7 @@ impl InferenceContext<'_> {
self.walk_pat_inner(
pat,
&mut update_result,
BorrowKind::Mut { allow_two_phase_borrow: false },
BorrowKind::Mut { kind: MutBorrowKind::Default },
);
}

Expand Down Expand Up @@ -699,7 +701,7 @@ impl InferenceContext<'_> {
},
}
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));
}
Expand Down Expand Up @@ -880,7 +882,7 @@ impl InferenceContext<'_> {
}
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());
Expand Down Expand Up @@ -930,9 +932,7 @@ impl InferenceContext<'_> {
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,
},
Expand All @@ -949,8 +949,12 @@ impl InferenceContext<'_> {
};
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.
Expand Down
61 changes: 14 additions & 47 deletions crates/hir-ty/src/mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
}
}
}
Expand Down
12 changes: 9 additions & 3 deletions crates/hir-ty/src/mir/borrowck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use crate::{
};

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)]
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion crates/hir-ty/src/mir/lower/as_place.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! MIR lowering for places

use crate::mir::MutBorrowKind;

use super::*;
use hir_def::FunctionId;
use hir_expand::name;
Expand Down Expand Up @@ -328,7 +330,7 @@ impl MirLowerCtx<'_> {
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);
Expand Down
17 changes: 10 additions & 7 deletions crates/hir-ty/src/mir/lower/pattern_matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -450,7 +453,7 @@ impl MirLowerCtx<'_> {
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,
Expand Down
9 changes: 6 additions & 3 deletions crates/hir-ty/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ use crate::{
};

use super::{
AggregateKind, BasicBlockId, BorrowKind, LocalId, MirBody, Operand, Place, Rvalue, UnOp,
AggregateKind, BasicBlockId, BorrowKind, LocalId, MirBody, MutBorrowKind, Operand, Place,
Rvalue, UnOp,
};

macro_rules! w {
Expand Down Expand Up @@ -366,8 +367,10 @@ impl<'a> MirPrettyCtx<'a> {
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);
}
Expand Down
14 changes: 7 additions & 7 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ use hir_ty::{
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,
Expand Down Expand Up @@ -3754,12 +3754,12 @@ impl ClosureCapture {
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,
}
}
Expand Down
Loading