Skip to content

Commit 364bf39

Browse files
borsbors
committed
Auto merge of #94775 - oli-obk:operand_order, r=davidtwco
Fix constants not getting dropped if part of a diverging expression fixes #90762 cc `@RalfJung`
2 parents 2567420 + db02e61 commit 364bf39

File tree

8 files changed

+144
-34
lines changed

8 files changed

+144
-34
lines changed

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

+9-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! See docs in build/expr/mod.rs
22
33
use crate::build::expr::category::Category;
4-
use crate::build::{BlockAnd, BlockAndExtension, Builder};
4+
use crate::build::{BlockAnd, BlockAndExtension, Builder, NeedsTemporary};
55
use rustc_middle::middle::region;
66
use rustc_middle::mir::*;
77
use rustc_middle::thir::*;
@@ -20,7 +20,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
2020
expr: &Expr<'tcx>,
2121
) -> BlockAnd<Operand<'tcx>> {
2222
let local_scope = self.local_scope();
23-
self.as_operand(block, Some(local_scope), expr, None)
23+
self.as_operand(block, Some(local_scope), expr, None, NeedsTemporary::Maybe)
2424
}
2525

2626
/// Returns an operand suitable for use until the end of the current scope expression and
@@ -94,32 +94,33 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
9494
///
9595
/// Like `as_local_call_operand`, except that the argument will
9696
/// not be valid once `scope` ends.
97+
#[instrument(level = "debug", skip(self, scope))]
9798
crate fn as_operand(
9899
&mut self,
99100
mut block: BasicBlock,
100101
scope: Option<region::Scope>,
101102
expr: &Expr<'tcx>,
102103
local_info: Option<Box<LocalInfo<'tcx>>>,
104+
needs_temporary: NeedsTemporary,
103105
) -> BlockAnd<Operand<'tcx>> {
104-
debug!("as_operand(block={:?}, expr={:?} local_info={:?})", block, expr, local_info);
105106
let this = self;
106107

107108
if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
108109
let source_info = this.source_info(expr.span);
109110
let region_scope = (region_scope, source_info);
110111
return this.in_scope(region_scope, lint_level, |this| {
111-
this.as_operand(block, scope, &this.thir[value], local_info)
112+
this.as_operand(block, scope, &this.thir[value], local_info, needs_temporary)
112113
});
113114
}
114115

115116
let category = Category::of(&expr.kind).unwrap();
116-
debug!("as_operand: category={:?} for={:?}", category, expr.kind);
117+
debug!(?category, ?expr.kind);
117118
match category {
118-
Category::Constant => {
119+
Category::Constant if let NeedsTemporary::No = needs_temporary || !expr.ty.needs_drop(this.tcx, this.param_env) => {
119120
let constant = this.as_constant(expr);
120121
block.and(Operand::Constant(Box::new(constant)))
121122
}
122-
Category::Place | Category::Rvalue(..) => {
123+
Category::Constant | Category::Place | Category::Rvalue(..) => {
123124
let operand = unpack!(block = this.as_temp(block, scope, expr, Mutability::Mut));
124125
if this.local_decls[operand].local_info.is_none() {
125126
this.local_decls[operand].local_info = local_info;
@@ -176,6 +177,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
176177
}
177178
}
178179

179-
this.as_operand(block, scope, expr, None)
180+
this.as_operand(block, scope, expr, None, NeedsTemporary::Maybe)
180181
}
181182
}

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

+67-18
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use rustc_index::vec::Idx;
44

55
use crate::build::expr::as_place::PlaceBase;
66
use crate::build::expr::category::{Category, RvalueFunc};
7-
use crate::build::{BlockAnd, BlockAndExtension, Builder};
7+
use crate::build::{BlockAnd, BlockAndExtension, Builder, NeedsTemporary};
88
use rustc_hir::lang_items::LangItem;
99
use rustc_middle::middle::region;
1010
use rustc_middle::mir::AssertKind;
@@ -52,17 +52,28 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
5252
})
5353
}
5454
ExprKind::Repeat { value, count } => {
55-
let value_operand =
56-
unpack!(block = this.as_operand(block, scope, &this.thir[value], None));
55+
let value_operand = unpack!(
56+
block =
57+
this.as_operand(block, scope, &this.thir[value], None, NeedsTemporary::No)
58+
);
5759
block.and(Rvalue::Repeat(value_operand, count))
5860
}
5961
ExprKind::Binary { op, lhs, rhs } => {
60-
let lhs = unpack!(block = this.as_operand(block, scope, &this.thir[lhs], None));
61-
let rhs = unpack!(block = this.as_operand(block, scope, &this.thir[rhs], None));
62+
let lhs = unpack!(
63+
block =
64+
this.as_operand(block, scope, &this.thir[lhs], None, NeedsTemporary::Maybe)
65+
);
66+
let rhs = unpack!(
67+
block =
68+
this.as_operand(block, scope, &this.thir[rhs], None, NeedsTemporary::No)
69+
);
6270
this.build_binary_op(block, op, expr_span, expr.ty, lhs, rhs)
6371
}
6472
ExprKind::Unary { op, arg } => {
65-
let arg = unpack!(block = this.as_operand(block, scope, &this.thir[arg], None));
73+
let arg = unpack!(
74+
block =
75+
this.as_operand(block, scope, &this.thir[arg], None, NeedsTemporary::No)
76+
);
6677
// Check for -MIN on signed integers
6778
if this.check_overflow && op == UnOp::Neg && expr.ty.is_signed() {
6879
let bool_ty = this.tcx.types.bool;
@@ -167,13 +178,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
167178
block.and(Rvalue::Use(Operand::Move(Place::from(result))))
168179
}
169180
ExprKind::Cast { source } => {
170-
let source =
171-
unpack!(block = this.as_operand(block, scope, &this.thir[source], None));
181+
let source = unpack!(
182+
block =
183+
this.as_operand(block, scope, &this.thir[source], None, NeedsTemporary::No)
184+
);
172185
block.and(Rvalue::Cast(CastKind::Misc, source, expr.ty))
173186
}
174187
ExprKind::Pointer { cast, source } => {
175-
let source =
176-
unpack!(block = this.as_operand(block, scope, &this.thir[source], None));
188+
let source = unpack!(
189+
block =
190+
this.as_operand(block, scope, &this.thir[source], None, NeedsTemporary::No)
191+
);
177192
block.and(Rvalue::Cast(CastKind::Pointer(cast), source, expr.ty))
178193
}
179194
ExprKind::Array { ref fields } => {
@@ -208,7 +223,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
208223
let fields: Vec<_> = fields
209224
.into_iter()
210225
.copied()
211-
.map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f], None)))
226+
.map(|f| {
227+
unpack!(
228+
block = this.as_operand(
229+
block,
230+
scope,
231+
&this.thir[f],
232+
None,
233+
NeedsTemporary::Maybe
234+
)
235+
)
236+
})
212237
.collect();
213238

214239
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(el_ty)), fields))
@@ -219,7 +244,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
219244
let fields: Vec<_> = fields
220245
.into_iter()
221246
.copied()
222-
.map(|f| unpack!(block = this.as_operand(block, scope, &this.thir[f], None)))
247+
.map(|f| {
248+
unpack!(
249+
block = this.as_operand(
250+
block,
251+
scope,
252+
&this.thir[f],
253+
None,
254+
NeedsTemporary::Maybe
255+
)
256+
)
257+
})
223258
.collect();
224259

225260
block.and(Rvalue::Aggregate(Box::new(AggregateKind::Tuple), fields))
@@ -296,7 +331,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
296331
)
297332
),
298333
_ => {
299-
unpack!(block = this.as_operand(block, scope, upvar, None))
334+
unpack!(
335+
block = this.as_operand(
336+
block,
337+
scope,
338+
upvar,
339+
None,
340+
NeedsTemporary::Maybe
341+
)
342+
)
300343
}
301344
}
302345
}
@@ -325,13 +368,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
325368
literal: ConstantKind::zero_sized(this.tcx.types.unit),
326369
}))))
327370
}
328-
ExprKind::Yield { .. }
329-
| ExprKind::Literal { .. }
371+
372+
ExprKind::Literal { .. }
330373
| ExprKind::NamedConst { .. }
331374
| ExprKind::NonHirLiteral { .. }
332375
| ExprKind::ConstParam { .. }
333376
| ExprKind::ConstBlock { .. }
334-
| ExprKind::StaticRef { .. }
377+
| ExprKind::StaticRef { .. } => {
378+
let constant = this.as_constant(expr);
379+
block.and(Rvalue::Use(Operand::Constant(Box::new(constant))))
380+
}
381+
382+
ExprKind::Yield { .. }
335383
| ExprKind::Block { .. }
336384
| ExprKind::Match { .. }
337385
| ExprKind::If { .. }
@@ -359,9 +407,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
359407
// so make an operand and then return that
360408
debug_assert!(!matches!(
361409
Category::of(&expr.kind),
362-
Some(Category::Rvalue(RvalueFunc::AsRvalue))
410+
Some(Category::Rvalue(RvalueFunc::AsRvalue) | Category::Constant)
363411
));
364-
let operand = unpack!(block = this.as_operand(block, scope, expr, None));
412+
let operand =
413+
unpack!(block = this.as_operand(block, scope, expr, None, NeedsTemporary::No));
365414
block.and(Rvalue::Use(operand))
366415
}
367416
}

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

+12-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! See docs in build/expr/mod.rs
22
33
use crate::build::expr::category::{Category, RvalueFunc};
4-
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
4+
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, NeedsTemporary};
55
use rustc_ast::InlineAsmOptions;
66
use rustc_data_structures::fx::FxHashMap;
77
use rustc_data_structures::stack::ensure_sufficient_stack;
@@ -329,7 +329,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
329329
block,
330330
Some(scope),
331331
&this.thir[f.expr],
332-
Some(local_info)
332+
Some(local_info),
333+
NeedsTemporary::Maybe,
333334
)
334335
),
335336
)
@@ -516,8 +517,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
516517

517518
ExprKind::Yield { value } => {
518519
let scope = this.local_scope();
519-
let value =
520-
unpack!(block = this.as_operand(block, Some(scope), &this.thir[value], None));
520+
let value = unpack!(
521+
block = this.as_operand(
522+
block,
523+
Some(scope),
524+
&this.thir[value],
525+
None,
526+
NeedsTemporary::No
527+
)
528+
);
521529
let resume = this.cfg.start_new_block();
522530
this.cfg.terminate(
523531
block,

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

+12
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,18 @@ rustc_index::newtype_index! {
549549
struct ScopeId { .. }
550550
}
551551

552+
#[derive(Debug)]
553+
enum NeedsTemporary {
554+
/// Use this variant when whatever you are converting with `as_operand`
555+
/// is the last thing you are converting. This means that if we introduced
556+
/// an intermediate temporary, we'd only read it immediately after, so we can
557+
/// also avoid it.
558+
No,
559+
/// For all cases where you aren't sure or that are too expensive to compute
560+
/// for now. It is always safe to fall back to this.
561+
Maybe,
562+
}
563+
552564
///////////////////////////////////////////////////////////////////////////
553565
/// The `BlockAnd` "monad" packages up the new basic block along with a
554566
/// produced value (sometimes just unit, of course). The `unpack!`

‎compiler/rustc_mir_build/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#![feature(box_patterns)]
77
#![feature(control_flow_enum)]
88
#![feature(crate_visibility_modifier)]
9+
#![feature(if_let_guard)]
910
#![feature(let_chains)]
1011
#![feature(let_else)]
1112
#![feature(min_specialization)]

‎src/test/mir-opt/inline/inline_into_box_place.main.Inline.32bit.diff

+6-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
scope 2 {
1717
}
1818
+ scope 3 (inlined Vec::<u32>::new) { // at $DIR/inline-into-box-place.rs:8:33: 8:43
19+
+ let mut _8: alloc::raw_vec::RawVec<u32>; // in scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
1920
+ }
2021

2122
bb0: {
@@ -34,8 +35,8 @@
3435
- (*_5) = Vec::<u32>::new() -> [return: bb2, unwind: bb5]; // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
3536
+ StorageLive(_7); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
3637
+ _7 = &mut (*_5); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
37-
+ Deinit((*_7)); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
38-
+ ((*_7).0: alloc::raw_vec::RawVec<u32>) = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: NonNull::<u32> { pointer: {0x4 as *const u32} }, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
38+
+ StorageLive(_8); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
39+
+ _8 = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: NonNull::<u32> { pointer: {0x4 as *const u32} }, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
3940
// mir::Constant
4041
- // + span: $DIR/inline-into-box-place.rs:8:33: 8:41
4142
- // + user_ty: UserType(1)
@@ -46,7 +47,10 @@
4647
+ // + span: $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
4748
+ // + user_ty: UserType(0)
4849
+ // + literal: Const { ty: alloc::raw_vec::RawVec<u32>, val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [255], len: Size { raw: 8 } }, align: Align { pow2: 2 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }) }
50+
+ Deinit((*_7)); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
51+
+ ((*_7).0: alloc::raw_vec::RawVec<u32>) = move _8; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
4952
+ ((*_7).1: usize) = const 0_usize; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
53+
+ StorageDead(_8); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
5054
+ StorageDead(_7); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
5155
_1 = move _5; // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43
5256
StorageDead(_5); // scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43

‎src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff

+6-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
scope 2 {
1717
}
1818
+ scope 3 (inlined Vec::<u32>::new) { // at $DIR/inline-into-box-place.rs:8:33: 8:43
19+
+ let mut _8: alloc::raw_vec::RawVec<u32>; // in scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
1920
+ }
2021

2122
bb0: {
@@ -34,8 +35,8 @@
3435
- (*_5) = Vec::<u32>::new() -> [return: bb2, unwind: bb5]; // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
3536
+ StorageLive(_7); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
3637
+ _7 = &mut (*_5); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
37-
+ Deinit((*_7)); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
38-
+ ((*_7).0: alloc::raw_vec::RawVec<u32>) = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: NonNull::<u32> { pointer: {0x4 as *const u32} }, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
38+
+ StorageLive(_8); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
39+
+ _8 = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: NonNull::<u32> { pointer: {0x4 as *const u32} }, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
3940
// mir::Constant
4041
- // + span: $DIR/inline-into-box-place.rs:8:33: 8:41
4142
- // + user_ty: UserType(1)
@@ -46,7 +47,10 @@
4647
+ // + span: $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
4748
+ // + user_ty: UserType(0)
4849
+ // + literal: Const { ty: alloc::raw_vec::RawVec<u32>, val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [65535], len: Size { raw: 16 } }, align: Align { pow2: 3 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }) }
50+
+ Deinit((*_7)); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
51+
+ ((*_7).0: alloc::raw_vec::RawVec<u32>) = move _8; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
4952
+ ((*_7).1: usize) = const 0_usize; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
53+
+ StorageDead(_8); // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
5054
+ StorageDead(_7); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
5155
_1 = move _5; // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43
5256
StorageDead(_5); // scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43

‎src/test/ui/consts/issue-90762.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// run-pass
2+
#![allow(unreachable_code)]
3+
4+
use std::sync::atomic::{AtomicBool, Ordering, AtomicUsize};
5+
6+
struct Print(usize);
7+
8+
impl Drop for Print {
9+
fn drop(&mut self) {
10+
println!("{}", self.0);
11+
FOO[self.0].store(true, Ordering::Relaxed);
12+
assert_eq!(BAR.fetch_sub(1, Ordering::Relaxed), self.0);
13+
}
14+
}
15+
16+
const A: Print = Print(0);
17+
const B: Print = Print(1);
18+
19+
static FOO: [AtomicBool; 3] =
20+
[AtomicBool::new(false), AtomicBool::new(false), AtomicBool::new(false)];
21+
static BAR: AtomicUsize = AtomicUsize::new(2);
22+
23+
fn main() {
24+
loop {
25+
std::mem::forget(({ A }, B, Print(2), break));
26+
}
27+
for (i, b) in FOO.iter().enumerate() {
28+
assert!(b.load(Ordering::Relaxed), "{} not set", i);
29+
}
30+
assert_eq!(BAR.fetch_add(1, Ordering::Relaxed), usize::max_value());
31+
}

0 commit comments

Comments
 (0)