Skip to content

Commit

Permalink
Auto merge of rust-lang#117712 - lcnr:expand-coroutine, r=jackh726
Browse files Browse the repository at this point in the history
generator layout: ignore fake borrows

fixes rust-lang#117059

We emit fake shallow borrows in case the scrutinee place uses a `Deref` and there is a match guard. This is necessary to prevent the match guard from mutating the scrutinee: https://github.com/rust-lang/rust/blob/fab1054e1742790c22ccc92a625736d658363677/compiler/rustc_mir_build/src/build/matches/mod.rs#L1250-L1265

These fake borrows end up impacting the generator witness computation in `mir_generator_witnesses`, which causes the issue in rust-lang#117059. This PR now completely ignores fake borrows during this computation. This is sound as thse are always removed after analysis and the actual computation of the generator layout happens afterwards.

Only the second commit impacts behavior, and could be backported by itself.

r? types
  • Loading branch information
bors committed Nov 9, 2023
2 parents 287ae4d + 92267c9 commit 14575e5
Show file tree
Hide file tree
Showing 40 changed files with 140 additions and 91 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ 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::Shallow => "shallow ",
mir::BorrowKind::Fake => "fake ",
mir::BorrowKind::Mut { kind: mir::MutBorrowKind::ClosureCapture } => "uniq ",
// FIXME: differentiate `TwoPhaseBorrow`
mir::BorrowKind::Mut {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/def_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {
// cross suspension points so this behavior is unproblematic.
PlaceContext::MutatingUse(MutatingUseContext::Borrow) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::FakeBorrow) |

// `PlaceMention` and `AscribeUserType` both evaluate the place, which must not
// contain dangling references.
Expand Down
13 changes: 6 additions & 7 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
self.cannot_uniquely_borrow_by_two_closures(span, &desc_place, issued_span, None)
}

(BorrowKind::Mut { .. }, BorrowKind::Shallow) => {
(BorrowKind::Mut { .. }, BorrowKind::Fake) => {
if let Some(immutable_section_description) =
self.classify_immutable_section(issued_borrow.assigned_place)
{
Expand Down Expand Up @@ -1114,11 +1114,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
)
}

(BorrowKind::Shared, BorrowKind::Shared | BorrowKind::Shallow)
| (
BorrowKind::Shallow,
BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Shallow,
) => unreachable!(),
(BorrowKind::Shared, BorrowKind::Shared | BorrowKind::Fake)
| (BorrowKind::Fake, BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake) => {
unreachable!()
}
};

if issued_spans == borrow_spans {
Expand Down Expand Up @@ -2806,7 +2805,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::Shallow {
if loan.kind == BorrowKind::Fake {
if let Some(section) = self.classify_immutable_section(loan.assigned_place) {
let mut err = self.cannot_mutate_in_immutable_section(
span,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ impl UseSpans<'_> {
err.subdiagnostic(match kind {
Some(kd) => match kd {
rustc_middle::mir::BorrowKind::Shared
| rustc_middle::mir::BorrowKind::Shallow => {
| rustc_middle::mir::BorrowKind::Fake => {
CaptureVarKind::Immut { kind_span: capture_kind_span }
}

Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_borrowck/src/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,8 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
match rvalue {
&Rvalue::Ref(_ /*rgn*/, bk, place) => {
let access_kind = match bk {
BorrowKind::Shallow => {
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
BorrowKind::Fake => {
(Shallow(Some(ArtificialField::FakeBorrow)), Read(ReadKind::Borrow(bk)))
}
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
BorrowKind::Mut { .. } => {
Expand Down Expand Up @@ -376,8 +376,8 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
// have already taken the reservation
}

(Read(_), BorrowKind::Shallow | BorrowKind::Shared)
| (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
(Read(_), BorrowKind::Fake | BorrowKind::Shared)
| (Read(ReadKind::Borrow(BorrowKind::Fake)), BorrowKind::Mut { .. }) => {
// Reads don't invalidate shared or shallow borrows
}

Expand Down Expand Up @@ -422,7 +422,7 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {

// only mutable borrows should be 2-phase
assert!(match borrow.kind {
BorrowKind::Shared | BorrowKind::Shallow => false,
BorrowKind::Shared | BorrowKind::Fake => false,
BorrowKind::Mut { .. } => true,
});

Expand Down
24 changes: 12 additions & 12 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ use self::ReadOrWrite::{Activation, Read, Reservation, Write};
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
enum ArtificialField {
ArrayLength,
ShallowBorrow,
FakeBorrow,
}

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -1085,18 +1085,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Control::Continue
}

(Read(_), BorrowKind::Shared | BorrowKind::Shallow)
| (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
(Read(_), BorrowKind::Shared | BorrowKind::Fake)
| (Read(ReadKind::Borrow(BorrowKind::Fake)), BorrowKind::Mut { .. }) => {
Control::Continue
}

(Reservation(_), BorrowKind::Shallow | 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::Shallow) => {
(Write(WriteKind::Move), BorrowKind::Fake) => {
// Handled by initialization checks.
Control::Continue
}
Expand Down Expand Up @@ -1204,8 +1204,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
match rvalue {
&Rvalue::Ref(_ /*rgn*/, bk, place) => {
let access_kind = match bk {
BorrowKind::Shallow => {
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
BorrowKind::Fake => {
(Shallow(Some(ArtificialField::FakeBorrow)), Read(ReadKind::Borrow(bk)))
}
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
BorrowKind::Mut { .. } => {
Expand All @@ -1226,7 +1226,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
flow_state,
);

let action = if bk == BorrowKind::Shallow {
let action = if bk == BorrowKind::Fake {
InitializationRequiringAction::MatchOn
} else {
InitializationRequiringAction::Borrow
Expand Down Expand Up @@ -1583,7 +1583,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

// only mutable borrows should be 2-phase
assert!(match borrow.kind {
BorrowKind::Shared | BorrowKind::Shallow => false,
BorrowKind::Shared | BorrowKind::Fake => false,
BorrowKind::Mut { .. } => true,
});

Expand Down Expand Up @@ -2142,14 +2142,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
| WriteKind::Replace
| WriteKind::StorageDeadOrDrop
| WriteKind::MutableBorrow(BorrowKind::Shared)
| WriteKind::MutableBorrow(BorrowKind::Shallow),
| WriteKind::MutableBorrow(BorrowKind::Fake),
)
| Write(
WriteKind::Move
| WriteKind::Replace
| WriteKind::StorageDeadOrDrop
| WriteKind::MutableBorrow(BorrowKind::Shared)
| WriteKind::MutableBorrow(BorrowKind::Shallow),
| WriteKind::MutableBorrow(BorrowKind::Fake),
) => {
if self.is_mutable(place.as_ref(), is_local_mutation_allowed).is_err()
&& !self.has_buffered_errors()
Expand All @@ -2173,7 +2173,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
return false;
}
Read(
ReadKind::Borrow(BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Shallow)
ReadKind::Borrow(BorrowKind::Mut { .. } | BorrowKind::Shared | BorrowKind::Fake)
| ReadKind::Copy,
) => {
// Access authorized
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/places_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ fn place_components_conflict<'tcx>(

match (elem, &base_ty.kind(), access) {
(_, _, Shallow(Some(ArtificialField::ArrayLength)))
| (_, _, Shallow(Some(ArtificialField::ShallowBorrow))) => {
| (_, _, Shallow(Some(ArtificialField::FakeBorrow))) => {
// The array length is like additional fields on the
// type; it does not overlap any existing data there.
// Furthermore, if cannot actually be a prefix of any
Expand Down Expand Up @@ -273,10 +273,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::Shallow
if borrow_kind == BorrowKind::Fake
&& borrow_place.projection.len() < access_place.projection.len()
{
debug!("borrow_conflicts_with_place: shallow borrow");
debug!("borrow_conflicts_with_place: fake borrow");
false
} else {
debug!("borrow_conflicts_with_place: full borrow, CONFLICT");
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
PlaceContext::MutatingUse(_) => ty::Invariant,
PlaceContext::NonUse(StorageDead | StorageLive | VarDebugInfo) => ty::Invariant,
PlaceContext::NonMutatingUse(
Inspect | Copy | Move | PlaceMention | SharedBorrow | ShallowBorrow | AddressOf
Inspect | Copy | Move | PlaceMention | SharedBorrow | FakeBorrow | AddressOf
| Projection,
) => ty::Covariant,
PlaceContext::NonUse(AscribeUserTy(variance)) => variance,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
| PlaceContext::NonMutatingUse(
NonMutatingUseContext::Inspect
| NonMutatingUseContext::SharedBorrow
| NonMutatingUseContext::ShallowBorrow
| NonMutatingUseContext::FakeBorrow
| NonMutatingUseContext::AddressOf
| NonMutatingUseContext::Projection,
) => {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
BorrowKind::Shared => {
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow)
}
BorrowKind::Shallow => {
PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow)
BorrowKind::Fake => {
PlaceContext::NonMutatingUse(NonMutatingUseContext::FakeBorrow)
}
BorrowKind::Mut { .. } => {
PlaceContext::MutatingUse(MutatingUseContext::Borrow)
Expand Down Expand Up @@ -500,7 +500,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
self.check_mut_borrow(place.local, hir::BorrowKind::Raw)
}

Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Shallow, place)
Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Fake, place)
| Rvalue::AddressOf(Mutability::Not, place) => {
let borrowed_place_has_mut_interior = qualifs::in_place::<HasMutInterior, _>(
&self.ccx,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::Shallow => {
mir::BorrowKind::Shared | mir::BorrowKind::Fake => {
self.shared_borrow_allows_mutation(place)
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,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::Shallow | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => {
BorrowKind::Fake | BorrowKind::Mut { kind: MutBorrowKind::ClosureCapture } => {
return Err(Unpromotable);
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -848,11 +848,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
match rvalue {
Rvalue::Use(_) | Rvalue::CopyForDeref(_) | Rvalue::Aggregate(..) => {}
Rvalue::Ref(_, BorrowKind::Shallow, _) => {
Rvalue::Ref(_, BorrowKind::Fake, _) => {
if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) {
self.fail(
location,
"`Assign` statement with a `Shallow` borrow should have been removed in runtime MIR",
"`Assign` statement with a `Fake` borrow should have been removed in runtime MIR",
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
Ref(region, borrow_kind, ref place) => {
let kind_str = match borrow_kind {
BorrowKind::Shared => "",
BorrowKind::Shallow => "shallow ",
BorrowKind::Fake => "fake ",
BorrowKind::Mut { .. } => "mut ",
};

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,15 +446,15 @@ impl<'tcx> Rvalue<'tcx> {
impl BorrowKind {
pub fn mutability(&self) -> Mutability {
match *self {
BorrowKind::Shared | BorrowKind::Shallow => Mutability::Not,
BorrowKind::Shared | BorrowKind::Fake => Mutability::Not,
BorrowKind::Mut { .. } => Mutability::Mut,
}
}

pub fn allows_two_phase_borrow(&self) -> bool {
match *self {
BorrowKind::Shared
| BorrowKind::Shallow
| BorrowKind::Fake
| BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::ClosureCapture } => {
false
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub enum AnalysisPhase {
/// * [`TerminatorKind::FalseEdge`]
/// * [`StatementKind::FakeRead`]
/// * [`StatementKind::AscribeUserType`]
/// * [`Rvalue::Ref`] with `BorrowKind::Shallow`
/// * [`Rvalue::Ref`] with `BorrowKind::Fake`
///
/// Furthermore, `Deref` projections must be the first projection within any place (if they
/// appear at all)
Expand Down Expand Up @@ -182,7 +182,7 @@ pub enum BorrowKind {
/// 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.
Shallow,
Fake,

/// Data is mutable and not aliasable.
Mut { kind: MutBorrowKind },
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ impl BorrowKind {

// We have no type corresponding to a shallow borrow, so use
// `&` as an approximation.
BorrowKind::Shallow => hir::Mutability::Not,
BorrowKind::Fake => hir::Mutability::Not,
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,8 +649,8 @@ macro_rules! make_mir_visitor {
BorrowKind::Shared => PlaceContext::NonMutatingUse(
NonMutatingUseContext::SharedBorrow
),
BorrowKind::Shallow => PlaceContext::NonMutatingUse(
NonMutatingUseContext::ShallowBorrow
BorrowKind::Fake => PlaceContext::NonMutatingUse(
NonMutatingUseContext::FakeBorrow
),
BorrowKind::Mut { .. } =>
PlaceContext::MutatingUse(MutatingUseContext::Borrow),
Expand Down Expand Up @@ -1261,8 +1261,8 @@ pub enum NonMutatingUseContext {
Move,
/// Shared borrow.
SharedBorrow,
/// Shallow borrow.
ShallowBorrow,
/// A fake borrow.
FakeBorrow,
/// AddressOf for *const pointer.
AddressOf,
/// PlaceMention statement.
Expand Down Expand Up @@ -1341,7 +1341,7 @@ impl PlaceContext {
matches!(
self,
PlaceContext::NonMutatingUse(
NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::ShallowBorrow
NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::FakeBorrow
) | PlaceContext::MutatingUse(MutatingUseContext::Borrow)
)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/as_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fake_borrow_temp.into(),
Rvalue::Ref(
tcx.lifetimes.re_erased,
BorrowKind::Shallow,
BorrowKind::Fake,
Place { local: base_place.local, projection },
),
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2021,7 +2021,7 @@ 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::Shallow, place);
let borrow = Rvalue::Ref(re_erased, BorrowKind::Fake, place);
self.cfg.push_assign(block, scrutinee_source_info, Place::from(temp), borrow);
}

Expand Down
Loading

0 comments on commit 14575e5

Please sign in to comment.