Skip to content

Commit 14c54b6

Browse files
borsbors
committed
Auto merge of #107844 - Zeegomo:no-drop-and-rep, r=cjgillot
Desugaring of drop and replace at MIR build This commit desugars the drop and replace deriving from an assignment at MIR build, avoiding the construction of the `DropAndReplace` terminator (which will be removed in a following PR). In order to retain the same error messages for replaces a new `DesugaringKind::Replace` variant is introduced. The changes in the borrowck are also useful for future work in moving drop elaboration before borrowck, as no `DropAndReplace` would be present there anymore. Notes on test diffs: * `tests/ui/borrowck/issue-58776-borrowck-scans-children`: the assignment deriving from the desugaring kills the borrow. * `tests/ui/async-await/async-fn-size-uninit-locals.rs`, `tests/mir-opt/issue_41110.test.ElaborateDrops.after.mir`, `tests/mir-opt/issue_41888.main.ElaborateDrops.after.mir`: drop elaboration generates (or reads from) a useless drop flag due to an issue with the dataflow analysis. Will be fixed independently by #106430. See #104488 for more context
2 parents 35636f9 + d1f7fa5 commit 14c54b6

22 files changed

+281
-152
lines changed

‎compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,6 +1467,32 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
14671467

14681468
/// Reports StorageDeadOrDrop of `place` conflicts with `borrow`.
14691469
///
1470+
/// Depending on the origin of the StorageDeadOrDrop, this may be
1471+
/// reported as either a drop or an illegal mutation of a borrowed value.
1472+
/// The latter is preferred when the this is a drop triggered by a
1473+
/// reassignment, as it's more user friendly to report a problem with the
1474+
/// explicit assignment than the implicit drop.
1475+
#[instrument(level = "debug", skip(self))]
1476+
pub(crate) fn report_storage_dead_or_drop_of_borrowed(
1477+
&mut self,
1478+
location: Location,
1479+
place_span: (Place<'tcx>, Span),
1480+
borrow: &BorrowData<'tcx>,
1481+
) {
1482+
// It's sufficient to check the last desugaring as Replace is the last
1483+
// one to be applied.
1484+
if let Some(DesugaringKind::Replace) = place_span.1.desugaring_kind() {
1485+
self.report_illegal_mutation_of_borrowed(location, place_span, borrow)
1486+
} else {
1487+
self.report_borrowed_value_does_not_live_long_enough(
1488+
location,
1489+
borrow,
1490+
place_span,
1491+
Some(WriteKind::StorageDeadOrDrop),
1492+
)
1493+
}
1494+
}
1495+
14701496
/// This means that some data referenced by `borrow` needs to live
14711497
/// past the point where the StorageDeadOrDrop of `place` occurs.
14721498
/// This is usually interpreted as meaning that `place` has too

‎compiler/rustc_borrowck/src/diagnostics/mod.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
925925
return OtherUse(use_span);
926926
}
927927

928-
for stmt in &self.body[location.block].statements[location.statement_index + 1..] {
928+
// drop and replace might have moved the assignment to the next block
929+
let maybe_additional_statement =
930+
if let TerminatorKind::Drop { target: drop_target, .. } =
931+
self.body[location.block].terminator().kind
932+
{
933+
self.body[drop_target].statements.first()
934+
} else {
935+
None
936+
};
937+
938+
let statements =
939+
self.body[location.block].statements[location.statement_index + 1..].iter();
940+
941+
for stmt in statements.chain(maybe_additional_statement) {
929942
if let StatementKind::Assign(box (_, Rvalue::Aggregate(kind, places))) = &stmt.kind {
930943
let (&def_id, is_generator) = match kind {
931944
box AggregateKind::Closure(def_id, _) => (def_id, false),

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
828828
let Some(hir::Node::Item(item)) = node else { return; };
829829
let hir::ItemKind::Fn(.., body_id) = item.kind else { return; };
830830
let body = self.infcx.tcx.hir().body(body_id);
831-
let mut v = V { assign_span: span, err, ty, suggested: false };
831+
let mut assign_span = span;
832+
// Drop desugaring is done at MIR build so it's not in the HIR
833+
if let Some(DesugaringKind::Replace) = span.desugaring_kind() {
834+
assign_span.remove_mark();
835+
}
836+
837+
let mut v = V { assign_span, err, ty, suggested: false };
832838
v.visit_body(body);
833839
if !v.suggested {
834840
err.help(&format!(

‎compiler/rustc_borrowck/src/lib.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,12 +1185,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
11851185
this.buffer_error(err);
11861186
}
11871187
WriteKind::StorageDeadOrDrop => this
1188-
.report_borrowed_value_does_not_live_long_enough(
1189-
location,
1190-
borrow,
1191-
place_span,
1192-
Some(kind),
1193-
),
1188+
.report_storage_dead_or_drop_of_borrowed(location, place_span, borrow),
11941189
WriteKind::Mutate => {
11951190
this.report_illegal_mutation_of_borrowed(location, place_span, borrow)
11961191
}

‎compiler/rustc_mir_build/src/build/expr/stmt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
4040
// Generate better code for things that don't need to be
4141
// dropped.
4242
if lhs.ty.needs_drop(this.tcx, this.param_env) {
43-
let rhs = unpack!(block = this.as_local_operand(block, rhs));
43+
let rhs = unpack!(block = this.as_local_rvalue(block, rhs));
4444
let lhs = unpack!(block = this.as_place(block, lhs));
4545
unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs));
4646
} else {

‎compiler/rustc_mir_build/src/build/scope.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ use rustc_middle::middle::region;
9191
use rustc_middle::mir::*;
9292
use rustc_middle::thir::{Expr, LintLevel};
9393

94-
use rustc_span::{Span, DUMMY_SP};
94+
use rustc_span::{DesugaringKind, Span, DUMMY_SP};
9595

9696
#[derive(Debug)]
9797
pub struct Scopes<'tcx> {
@@ -1118,24 +1118,35 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
11181118
}
11191119

11201120
/// Utility function for *non*-scope code to build their own drops
1121+
/// Force a drop at this point in the MIR by creating a new block.
11211122
pub(crate) fn build_drop_and_replace(
11221123
&mut self,
11231124
block: BasicBlock,
11241125
span: Span,
11251126
place: Place<'tcx>,
1126-
value: Operand<'tcx>,
1127+
value: Rvalue<'tcx>,
11271128
) -> BlockAnd<()> {
1129+
let span = self.tcx.with_stable_hashing_context(|hcx| {
1130+
span.mark_with_reason(None, DesugaringKind::Replace, self.tcx.sess.edition(), hcx)
1131+
});
11281132
let source_info = self.source_info(span);
1129-
let next_target = self.cfg.start_new_block();
1133+
1134+
// create the new block for the assignment
1135+
let assign = self.cfg.start_new_block();
1136+
self.cfg.push_assign(assign, source_info, place, value.clone());
1137+
1138+
// create the new block for the assignment in the case of unwinding
1139+
let assign_unwind = self.cfg.start_new_cleanup_block();
1140+
self.cfg.push_assign(assign_unwind, source_info, place, value.clone());
11301141

11311142
self.cfg.terminate(
11321143
block,
11331144
source_info,
1134-
TerminatorKind::DropAndReplace { place, value, target: next_target, unwind: None },
1145+
TerminatorKind::Drop { place, target: assign, unwind: Some(assign_unwind) },
11351146
);
11361147
self.diverge_from(block);
11371148

1138-
next_target.unit()
1149+
assign.unit()
11391150
}
11401151

11411152
/// Creates an `Assert` terminator and return the success block.
@@ -1413,8 +1424,15 @@ impl<'tcx> DropTreeBuilder<'tcx> for Unwind {
14131424
fn add_entry(cfg: &mut CFG<'tcx>, from: BasicBlock, to: BasicBlock) {
14141425
let term = &mut cfg.block_data_mut(from).terminator_mut();
14151426
match &mut term.kind {
1416-
TerminatorKind::Drop { unwind, .. }
1417-
| TerminatorKind::DropAndReplace { unwind, .. }
1427+
TerminatorKind::Drop { unwind, .. } => {
1428+
if let Some(unwind) = *unwind {
1429+
let source_info = term.source_info;
1430+
cfg.terminate(unwind, source_info, TerminatorKind::Goto { target: to });
1431+
} else {
1432+
*unwind = Some(to);
1433+
}
1434+
}
1435+
TerminatorKind::DropAndReplace { unwind, .. }
14181436
| TerminatorKind::FalseUnwind { unwind, .. }
14191437
| TerminatorKind::Call { cleanup: unwind, .. }
14201438
| TerminatorKind::Assert { cleanup: unwind, .. }

‎compiler/rustc_mir_transform/src/elaborate_drops.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use rustc_mir_dataflow::un_derefer::UnDerefer;
1414
use rustc_mir_dataflow::MoveDataParamEnv;
1515
use rustc_mir_dataflow::{on_all_children_bits, on_all_drop_children_bits};
1616
use rustc_mir_dataflow::{Analysis, ResultsCursor};
17-
use rustc_span::Span;
17+
use rustc_span::{DesugaringKind, Span};
1818
use rustc_target::abi::VariantIdx;
1919
use std::fmt;
2020

@@ -425,10 +425,19 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
425425
bb,
426426
),
427427
LookupResult::Parent(..) => {
428-
self.tcx.sess.delay_span_bug(
429-
terminator.source_info.span,
430-
&format!("drop of untracked value {:?}", bb),
431-
);
428+
if !matches!(
429+
terminator.source_info.span.desugaring_kind(),
430+
Some(DesugaringKind::Replace),
431+
) {
432+
self.tcx.sess.delay_span_bug(
433+
terminator.source_info.span,
434+
&format!("drop of untracked value {:?}", bb),
435+
);
436+
}
437+
// A drop and replace behind a pointer/array/whatever.
438+
// The borrow checker requires that these locations are initialized before the assignment,
439+
// so we just leave an unconditional drop.
440+
assert!(!data.is_cleanup);
432441
}
433442
}
434443
}

‎compiler/rustc_span/src/hygiene.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,6 +1151,7 @@ pub enum DesugaringKind {
11511151
Await,
11521152
ForLoop,
11531153
WhileLoop,
1154+
Replace,
11541155
}
11551156

11561157
impl DesugaringKind {
@@ -1166,6 +1167,7 @@ impl DesugaringKind {
11661167
DesugaringKind::OpaqueTy => "`impl Trait`",
11671168
DesugaringKind::ForLoop => "`for` loop",
11681169
DesugaringKind::WhileLoop => "`while` loop",
1170+
DesugaringKind::Replace => "drop and replace",
11691171
}
11701172
}
11711173
}

‎tests/mir-opt/basic_assignment.main.ElaborateDrops.diff

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
- // MIR for `main` before ElaborateDrops
2+
+ // MIR for `main` after ElaborateDrops
3+
4+
fn main() -> () {
5+
let mut _0: (); // return place in scope 0 at $DIR/basic_assignment.rs:+0:11: +0:11
6+
let _1: bool; // in scope 0 at $DIR/basic_assignment.rs:+1:9: +1:17
7+
let mut _3: bool; // in scope 0 at $DIR/basic_assignment.rs:+6:16: +6:24
8+
let mut _6: std::option::Option<std::boxed::Box<u32>>; // in scope 0 at $DIR/basic_assignment.rs:+13:14: +13:20
9+
scope 1 {
10+
debug nodrop_x => _1; // in scope 1 at $DIR/basic_assignment.rs:+1:9: +1:17
11+
let _2: bool; // in scope 1 at $DIR/basic_assignment.rs:+2:9: +2:17
12+
scope 2 {
13+
debug nodrop_y => _2; // in scope 2 at $DIR/basic_assignment.rs:+2:9: +2:17
14+
let _4: std::option::Option<std::boxed::Box<u32>>; // in scope 2 at $DIR/basic_assignment.rs:+8:9: +8:15
15+
scope 3 {
16+
debug drop_x => _4; // in scope 3 at $DIR/basic_assignment.rs:+8:9: +8:15
17+
let _5: std::option::Option<std::boxed::Box<u32>>; // in scope 3 at $DIR/basic_assignment.rs:+9:9: +9:15
18+
scope 4 {
19+
debug drop_y => _5; // in scope 4 at $DIR/basic_assignment.rs:+9:9: +9:15
20+
}
21+
}
22+
}
23+
}
24+
25+
bb0: {
26+
StorageLive(_1); // scope 0 at $DIR/basic_assignment.rs:+1:9: +1:17
27+
_1 = const false; // scope 0 at $DIR/basic_assignment.rs:+1:20: +1:25
28+
StorageLive(_2); // scope 1 at $DIR/basic_assignment.rs:+2:9: +2:17
29+
StorageLive(_3); // scope 2 at $DIR/basic_assignment.rs:+6:16: +6:24
30+
_3 = _1; // scope 2 at $DIR/basic_assignment.rs:+6:16: +6:24
31+
_2 = move _3; // scope 2 at $DIR/basic_assignment.rs:+6:5: +6:24
32+
StorageDead(_3); // scope 2 at $DIR/basic_assignment.rs:+6:23: +6:24
33+
StorageLive(_4); // scope 2 at $DIR/basic_assignment.rs:+8:9: +8:15
34+
_4 = Option::<Box<u32>>::None; // scope 2 at $DIR/basic_assignment.rs:+8:36: +8:40
35+
StorageLive(_5); // scope 3 at $DIR/basic_assignment.rs:+9:9: +9:15
36+
StorageLive(_6); // scope 4 at $DIR/basic_assignment.rs:+13:14: +13:20
37+
_6 = move _4; // scope 4 at $DIR/basic_assignment.rs:+13:14: +13:20
38+
- drop(_5) -> [return: bb1, unwind: bb2]; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
39+
+ goto -> bb1; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
40+
}
41+
42+
bb1: {
43+
_5 = move _6; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
44+
- drop(_6) -> [return: bb3, unwind: bb6]; // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
45+
+ goto -> bb3; // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
46+
}
47+
48+
bb2 (cleanup): {
49+
_5 = move _6; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
50+
drop(_6) -> bb6; // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
51+
}
52+
53+
bb3: {
54+
StorageDead(_6); // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
55+
_0 = const (); // scope 0 at $DIR/basic_assignment.rs:+0:11: +14:2
56+
drop(_5) -> [return: bb4, unwind: bb7]; // scope 3 at $DIR/basic_assignment.rs:+14:1: +14:2
57+
}
58+
59+
bb4: {
60+
StorageDead(_5); // scope 3 at $DIR/basic_assignment.rs:+14:1: +14:2
61+
- drop(_4) -> bb5; // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
62+
+ goto -> bb5; // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
63+
}
64+
65+
bb5: {
66+
StorageDead(_4); // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
67+
StorageDead(_2); // scope 1 at $DIR/basic_assignment.rs:+14:1: +14:2
68+
StorageDead(_1); // scope 0 at $DIR/basic_assignment.rs:+14:1: +14:2
69+
return; // scope 0 at $DIR/basic_assignment.rs:+14:2: +14:2
70+
}
71+
72+
bb6 (cleanup): {
73+
drop(_5) -> bb7; // scope 3 at $DIR/basic_assignment.rs:+14:1: +14:2
74+
}
75+
76+
bb7 (cleanup): {
77+
- drop(_4) -> bb8; // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
78+
+ goto -> bb8; // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
79+
}
80+
81+
bb8 (cleanup): {
82+
resume; // scope 0 at $DIR/basic_assignment.rs:+0:1: +14:2
83+
}
84+
}
85+

‎tests/mir-opt/basic_assignment.main.SimplifyCfg-initial.after.mir

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// MIR for `main` after SimplifyCfg-initial
22

33
| User Type Annotations
4-
| 0: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option<std::boxed::Box<u32>>) }, span: $DIR/basic_assignment.rs:18:17: 18:33, inferred_ty: std::option::Option<std::boxed::Box<u32>>
5-
| 1: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option<std::boxed::Box<u32>>) }, span: $DIR/basic_assignment.rs:18:17: 18:33, inferred_ty: std::option::Option<std::boxed::Box<u32>>
4+
| 0: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option<std::boxed::Box<u32>>) }, span: $DIR/basic_assignment.rs:20:17: 20:33, inferred_ty: std::option::Option<std::boxed::Box<u32>>
5+
| 1: user_ty: Canonical { max_universe: U0, variables: [], value: Ty(std::option::Option<std::boxed::Box<u32>>) }, span: $DIR/basic_assignment.rs:20:17: 20:33, inferred_ty: std::option::Option<std::boxed::Box<u32>>
66
|
77
fn main() -> () {
88
let mut _0: (); // return place in scope 0 at $DIR/basic_assignment.rs:+0:11: +0:11
@@ -41,35 +41,37 @@ fn main() -> () {
4141
StorageLive(_5); // scope 3 at $DIR/basic_assignment.rs:+9:9: +9:15
4242
StorageLive(_6); // scope 4 at $DIR/basic_assignment.rs:+13:14: +13:20
4343
_6 = move _4; // scope 4 at $DIR/basic_assignment.rs:+13:14: +13:20
44-
replace(_5 <- move _6) -> [return: bb1, unwind: bb5]; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
44+
drop(_5) -> [return: bb1, unwind: bb2]; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
4545
}
4646

4747
bb1: {
48-
drop(_6) -> [return: bb2, unwind: bb6]; // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
48+
_5 = move _6; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
49+
drop(_6) -> [return: bb3, unwind: bb6]; // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
4950
}
5051

51-
bb2: {
52+
bb2 (cleanup): {
53+
_5 = move _6; // scope 4 at $DIR/basic_assignment.rs:+13:5: +13:11
54+
drop(_6) -> bb6; // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
55+
}
56+
57+
bb3: {
5258
StorageDead(_6); // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
5359
_0 = const (); // scope 0 at $DIR/basic_assignment.rs:+0:11: +14:2
54-
drop(_5) -> [return: bb3, unwind: bb7]; // scope 3 at $DIR/basic_assignment.rs:+14:1: +14:2
60+
drop(_5) -> [return: bb4, unwind: bb7]; // scope 3 at $DIR/basic_assignment.rs:+14:1: +14:2
5561
}
5662

57-
bb3: {
63+
bb4: {
5864
StorageDead(_5); // scope 3 at $DIR/basic_assignment.rs:+14:1: +14:2
59-
drop(_4) -> [return: bb4, unwind: bb8]; // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
65+
drop(_4) -> [return: bb5, unwind: bb8]; // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
6066
}
6167

62-
bb4: {
68+
bb5: {
6369
StorageDead(_4); // scope 2 at $DIR/basic_assignment.rs:+14:1: +14:2
6470
StorageDead(_2); // scope 1 at $DIR/basic_assignment.rs:+14:1: +14:2
6571
StorageDead(_1); // scope 0 at $DIR/basic_assignment.rs:+14:1: +14:2
6672
return; // scope 0 at $DIR/basic_assignment.rs:+14:2: +14:2
6773
}
6874

69-
bb5 (cleanup): {
70-
drop(_6) -> bb6; // scope 4 at $DIR/basic_assignment.rs:+13:19: +13:20
71-
}
72-
7375
bb6 (cleanup): {
7476
drop(_5) -> bb7; // scope 3 at $DIR/basic_assignment.rs:+14:1: +14:2
7577
}

‎tests/mir-opt/basic_assignment.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
// needs-unwind
12
// this tests move up progration, which is not yet implemented
23

4+
// EMIT_MIR basic_assignment.main.ElaborateDrops.diff
35
// EMIT_MIR basic_assignment.main.SimplifyCfg-initial.after.mir
46

57
// Check codegen for assignments (`a = b`) where the left-hand-side is

0 commit comments

Comments
 (0)