Skip to content

Commit ca7d067

Browse files
borsbors
committed
Auto merge of #121786 - RalfJung:interior-mut-value-reasoning, r=<try>
const checking: do not do value-based reasoning for interior mutability This basically means a nicer error for #121610, see the new test in `tests/ui/consts/refs-to-cell-in-final.rs`. Currently we have a mismatch between the dynamic semantics of `&` (things might be mutable when the pointee type is `!Freeze`, no matter the pointee value) and const-qualif (doing value-based reasoning). This removes that mismatch. I don't think this can break anything that builds on current nightly; any such code would already later run into the error that the interner complains about the mutable pointer (that's exactly #121610). So a crater run makes little sense. Overall this peanuts compared to the *actual* problem with value-based reasoning for interior mutability in promotion (rust-lang/unsafe-code-guidelines#493). I have no idea how to resolve that. But at least now the use of the problematic qualif is down to 1... The first commit is independent, I just got worried about promotion of unions and was happy to notice that at least there we don't do value-based reasoning. r? `@oli-obk`
2 parents 4cdd205 + ac1ed22 commit ca7d067

File tree

8 files changed

+127
-38
lines changed

8 files changed

+127
-38
lines changed

‎compiler/rustc_const_eval/src/const_eval/eval_queries.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,10 @@ pub fn const_validate_mplace<'mir, 'tcx>(
412412
_ if cid.promoted.is_some() => CtfeValidationMode::Promoted,
413413
Some(mutbl) => CtfeValidationMode::Static { mutbl }, // a `static`
414414
None => {
415-
// In normal `const` (not promoted), the outermost allocation is always only copied,
416-
// so having `UnsafeCell` in there is okay despite them being in immutable memory.
417-
let allow_immutable_unsafe_cell = cid.promoted.is_none() && !inner;
418-
CtfeValidationMode::Const { allow_immutable_unsafe_cell }
415+
// This is a normal `const` (not promoted).
416+
// The outermost allocation is always only copied, so having `UnsafeCell` in there
417+
// is okay despite them being in immutable memory.
418+
CtfeValidationMode::Const { allow_immutable_unsafe_cell: !inner }
419419
}
420420
};
421421
ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?;

‎compiler/rustc_const_eval/src/transform/check_consts/check.rs‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type QualifResults<'mir, 'tcx, Q> =
3030
rustc_mir_dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'mir, 'mir, 'tcx, Q>>;
3131

3232
#[derive(Default)]
33-
pub struct Qualifs<'mir, 'tcx> {
33+
pub(crate) struct Qualifs<'mir, 'tcx> {
3434
has_mut_interior: Option<QualifResults<'mir, 'tcx, HasMutInterior>>,
3535
needs_drop: Option<QualifResults<'mir, 'tcx, NeedsDrop>>,
3636
needs_non_const_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>,
@@ -373,6 +373,21 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
373373
}
374374
}
375375
}
376+
377+
fn has_interior_mut(&self, ty: Ty<'tcx>) -> bool {
378+
match ty.kind() {
379+
// Empty arrays have no interior mutability no matter their element type.
380+
ty::Array(_elem, count)
381+
if count
382+
.try_eval_target_usize(self.tcx, self.param_env)
383+
.is_some_and(|v| v == 0) =>
384+
{
385+
false
386+
}
387+
// Fallback to checking `Freeze`.
388+
_ => !ty.is_freeze(self.tcx, self.param_env),
389+
}
390+
}
376391
}
377392

378393
impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
@@ -484,11 +499,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
484499

485500
Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Fake, place)
486501
| Rvalue::AddressOf(Mutability::Not, place) => {
487-
let borrowed_place_has_mut_interior = qualifs::in_place::<HasMutInterior, _>(
488-
self.ccx,
489-
&mut |local| self.qualifs.has_mut_interior(self.ccx, local, location),
490-
place.as_ref(),
491-
);
502+
// We don't do value-based reasoning here, since the rules for interior mutability
503+
// are not finalized yet and they seem likely to not be full value-based in the end.
504+
let borrowed_place_has_mut_interior =
505+
self.has_interior_mut(place.ty(self.body, self.tcx).ty);
492506

493507
// If the place is indirect, this is basically a reborrow. We have a reborrow
494508
// special case above, but for raw pointers and pointers/references to `static` and
@@ -498,6 +512,17 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
498512
// `TransientMutBorrow`), but such reborrows got accidentally stabilized already and
499513
// it is too much of a breaking change to take back.
500514
if borrowed_place_has_mut_interior && !place.is_indirect() {
515+
// We used to do a value-based check here, so when we changed to a purely
516+
// type-based check we started rejecting code that used to work on stable. So
517+
// for that reason we already stabilize "transient borrows of interior mutable
518+
// borrows where value-based reasoning says that there actually is no interior
519+
// mutability". We don't do anything like that for non-transient borrows since
520+
// those are and will remain hard errors.
521+
let allow_transient_on_stable = !qualifs::in_place::<HasMutInterior, _>(
522+
self.ccx,
523+
&mut |local| self.qualifs.has_mut_interior(self.ccx, local, location),
524+
place.as_ref(),
525+
);
501526
match self.const_kind() {
502527
// In a const fn all borrows are transient or point to the places given via
503528
// references in the arguments (so we already checked them with
@@ -506,7 +531,11 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
506531
// NOTE: Once we have heap allocations during CTFE we need to figure out
507532
// how to prevent `const fn` to create long-lived allocations that point
508533
// to (interior) mutable memory.
509-
hir::ConstContext::ConstFn => self.check_op(ops::TransientCellBorrow),
534+
hir::ConstContext::ConstFn => {
535+
if !allow_transient_on_stable {
536+
self.check_op(ops::TransientCellBorrow)
537+
}
538+
}
510539
_ => {
511540
// Locals with StorageDead are definitely not part of the final constant value, and
512541
// it is thus inherently safe to permit such locals to have their
@@ -517,7 +546,9 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
517546
// The good news is that interning will detect if any unexpected mutable
518547
// pointer slips through.
519548
if self.local_has_storage_dead(place.local) {
520-
self.check_op(ops::TransientCellBorrow);
549+
if !allow_transient_on_stable {
550+
self.check_op(ops::TransientCellBorrow);
551+
}
521552
} else {
522553
self.check_op(ops::CellBorrow);
523554
}

‎compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ where
284284
if Q::in_adt_inherently(cx, def, args) {
285285
return true;
286286
}
287+
// Don't do any value-based reasoning for unions.
287288
if def.is_union() && Q::in_any_value_of_ty(cx, rvalue.ty(cx.body, cx.tcx)) {
288289
return true;
289290
}

‎tests/ui/consts/enclosing-scope-rule.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//@build-pass
2+
// Some code that looks like it might be relying on promotion, but actually this is using the
3+
// enclosing-scope rule, meaning the reference is "extended" to outlive its block and live as long
4+
// as the surrounding block (which in this case is the entire program). There are multiple
5+
// allocations being interned at once.
6+
7+
struct Gen<T>(T);
8+
impl<'a, T> Gen<&'a T> {
9+
// Can't be promoted because `T` might not be `'static`.
10+
const C: &'a [T] = &[];
11+
}
12+
13+
// Can't be promoted because of `Drop`.
14+
const V: &Vec<i32> = &Vec::new();
15+
16+
fn main() {}

‎tests/ui/consts/promote-not.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#![allow(unconditional_panic)]
44

55
use std::cell::Cell;
6+
use std::mem::ManuallyDrop;
67

78
// We do not promote mutable references.
89
static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]); //~ ERROR temporary value dropped while borrowed
@@ -69,4 +70,12 @@ fn main() {
6970
let _val: &'static _ = &[&TEST_DROP; 1];
7071
//~^ ERROR temporary value dropped while borrowed
7172
//~| ERROR temporary value dropped while borrowed
73+
74+
// Make sure there is no value-based reasoning for unions.
75+
union UnionWithCell {
76+
f1: i32,
77+
f2: ManuallyDrop<Cell<i32>>,
78+
}
79+
let x: &'static _ = &UnionWithCell { f1: 0 };
80+
//~^ ERROR temporary value dropped while borrowed
7281
}

‎tests/ui/consts/promote-not.stderr‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0716]: temporary value dropped while borrowed
2-
--> $DIR/promote-not.rs:8:50
2+
--> $DIR/promote-not.rs:9:50
33
|
44
LL | static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]);
55
| ----------^^^^^^^^^-
@@ -9,7 +9,7 @@ LL | static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]);
99
| using this value as a static requires that borrow lasts for `'static`
1010

1111
error[E0716]: temporary value dropped while borrowed
12-
--> $DIR/promote-not.rs:11:18
12+
--> $DIR/promote-not.rs:12:18
1313
|
1414
LL | let x = &mut [1,2,3];
1515
| ^^^^^^^ creates a temporary value which is freed while still in use
@@ -19,7 +19,7 @@ LL | };
1919
| - temporary value is freed at the end of this statement
2020

2121
error[E0716]: temporary value dropped while borrowed
22-
--> $DIR/promote-not.rs:33:29
22+
--> $DIR/promote-not.rs:34:29
2323
|
2424
LL | let _x: &'static i32 = &unsafe { U { x: 0 }.x };
2525
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -29,7 +29,7 @@ LL | };
2929
| - temporary value is freed at the end of this statement
3030

3131
error[E0716]: temporary value dropped while borrowed
32-
--> $DIR/promote-not.rs:39:29
32+
--> $DIR/promote-not.rs:40:29
3333
|
3434
LL | let _val: &'static _ = &(Cell::new(1), 2).1;
3535
| ---------- ^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -39,7 +39,7 @@ LL | };
3939
| - temporary value is freed at the end of this statement
4040

4141
error[E0716]: temporary value dropped while borrowed
42-
--> $DIR/promote-not.rs:20:32
42+
--> $DIR/promote-not.rs:21:32
4343
|
4444
LL | let _x: &'static () = &foo();
4545
| ----------- ^^^^^ creates a temporary value which is freed while still in use
@@ -49,7 +49,7 @@ LL | }
4949
| - temporary value is freed at the end of this statement
5050

5151
error[E0716]: temporary value dropped while borrowed
52-
--> $DIR/promote-not.rs:28:29
52+
--> $DIR/promote-not.rs:29:29
5353
|
5454
LL | let _x: &'static i32 = &unsafe { U { x: 0 }.x };
5555
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -59,7 +59,7 @@ LL | }
5959
| - temporary value is freed at the end of this statement
6060

6161
error[E0716]: temporary value dropped while borrowed
62-
--> $DIR/promote-not.rs:46:29
62+
--> $DIR/promote-not.rs:47:29
6363
|
6464
LL | let _val: &'static _ = &(Cell::new(1), 2).0;
6565
| ---------- ^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -70,7 +70,7 @@ LL | }
7070
| - temporary value is freed at the end of this statement
7171

7272
error[E0716]: temporary value dropped while borrowed
73-
--> $DIR/promote-not.rs:47:29
73+
--> $DIR/promote-not.rs:48:29
7474
|
7575
LL | let _val: &'static _ = &(Cell::new(1), 2).1;
7676
| ---------- ^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -81,7 +81,7 @@ LL | }
8181
| - temporary value is freed at the end of this statement
8282

8383
error[E0716]: temporary value dropped while borrowed
84-
--> $DIR/promote-not.rs:50:29
84+
--> $DIR/promote-not.rs:51:29
8585
|
8686
LL | let _val: &'static _ = &(1/0);
8787
| ---------- ^^^^^ creates a temporary value which is freed while still in use
@@ -92,7 +92,7 @@ LL | }
9292
| - temporary value is freed at the end of this statement
9393

9494
error[E0716]: temporary value dropped while borrowed
95-
--> $DIR/promote-not.rs:51:29
95+
--> $DIR/promote-not.rs:52:29
9696
|
9797
LL | let _val: &'static _ = &(1/(1-1));
9898
| ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -103,7 +103,7 @@ LL | }
103103
| - temporary value is freed at the end of this statement
104104

105105
error[E0716]: temporary value dropped while borrowed
106-
--> $DIR/promote-not.rs:52:29
106+
--> $DIR/promote-not.rs:53:29
107107
|
108108
LL | let _val: &'static _ = &((1+1)/(1-1));
109109
| ---------- ^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -114,7 +114,7 @@ LL | }
114114
| - temporary value is freed at the end of this statement
115115

116116
error[E0716]: temporary value dropped while borrowed
117-
--> $DIR/promote-not.rs:53:29
117+
--> $DIR/promote-not.rs:54:29
118118
|
119119
LL | let _val: &'static _ = &(i32::MIN/-1);
120120
| ---------- ^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -125,7 +125,7 @@ LL | }
125125
| - temporary value is freed at the end of this statement
126126

127127
error[E0716]: temporary value dropped while borrowed
128-
--> $DIR/promote-not.rs:54:29
128+
--> $DIR/promote-not.rs:55:29
129129
|
130130
LL | let _val: &'static _ = &(i32::MIN/(0-1));
131131
| ---------- ^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -136,7 +136,7 @@ LL | }
136136
| - temporary value is freed at the end of this statement
137137

138138
error[E0716]: temporary value dropped while borrowed
139-
--> $DIR/promote-not.rs:55:29
139+
--> $DIR/promote-not.rs:56:29
140140
|
141141
LL | let _val: &'static _ = &(-128i8/-1);
142142
| ---------- ^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -147,7 +147,7 @@ LL | }
147147
| - temporary value is freed at the end of this statement
148148

149149
error[E0716]: temporary value dropped while borrowed
150-
--> $DIR/promote-not.rs:56:29
150+
--> $DIR/promote-not.rs:57:29
151151
|
152152
LL | let _val: &'static _ = &(1%0);
153153
| ---------- ^^^^^ creates a temporary value which is freed while still in use
@@ -158,7 +158,7 @@ LL | }
158158
| - temporary value is freed at the end of this statement
159159

160160
error[E0716]: temporary value dropped while borrowed
161-
--> $DIR/promote-not.rs:57:29
161+
--> $DIR/promote-not.rs:58:29
162162
|
163163
LL | let _val: &'static _ = &(1%(1-1));
164164
| ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -169,7 +169,7 @@ LL | }
169169
| - temporary value is freed at the end of this statement
170170

171171
error[E0716]: temporary value dropped while borrowed
172-
--> $DIR/promote-not.rs:58:29
172+
--> $DIR/promote-not.rs:59:29
173173
|
174174
LL | let _val: &'static _ = &([1,2,3][4]+1);
175175
| ---------- ^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -180,7 +180,7 @@ LL | }
180180
| - temporary value is freed at the end of this statement
181181

182182
error[E0716]: temporary value dropped while borrowed
183-
--> $DIR/promote-not.rs:61:29
183+
--> $DIR/promote-not.rs:62:29
184184
|
185185
LL | let _val: &'static _ = &TEST_DROP;
186186
| ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -191,7 +191,7 @@ LL | }
191191
| - temporary value is freed at the end of this statement
192192

193193
error[E0716]: temporary value dropped while borrowed
194-
--> $DIR/promote-not.rs:63:29
194+
--> $DIR/promote-not.rs:64:29
195195
|
196196
LL | let _val: &'static _ = &&TEST_DROP;
197197
| ---------- ^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -202,7 +202,7 @@ LL | }
202202
| - temporary value is freed at the end of this statement
203203

204204
error[E0716]: temporary value dropped while borrowed
205-
--> $DIR/promote-not.rs:63:30
205+
--> $DIR/promote-not.rs:64:30
206206
|
207207
LL | let _val: &'static _ = &&TEST_DROP;
208208
| ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -213,7 +213,7 @@ LL | }
213213
| - temporary value is freed at the end of this statement
214214

215215
error[E0716]: temporary value dropped while borrowed
216-
--> $DIR/promote-not.rs:66:29
216+
--> $DIR/promote-not.rs:67:29
217217
|
218218
LL | let _val: &'static _ = &(&TEST_DROP,);
219219
| ---------- ^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -224,7 +224,7 @@ LL | }
224224
| - temporary value is freed at the end of this statement
225225

226226
error[E0716]: temporary value dropped while borrowed
227-
--> $DIR/promote-not.rs:66:31
227+
--> $DIR/promote-not.rs:67:31
228228
|
229229
LL | let _val: &'static _ = &(&TEST_DROP,);
230230
| ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -235,7 +235,7 @@ LL | }
235235
| - temporary value is freed at the end of this statement
236236

237237
error[E0716]: temporary value dropped while borrowed
238-
--> $DIR/promote-not.rs:69:29
238+
--> $DIR/promote-not.rs:70:29
239239
|
240240
LL | let _val: &'static _ = &[&TEST_DROP; 1];
241241
| ---------- ^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -246,14 +246,25 @@ LL | }
246246
| - temporary value is freed at the end of this statement
247247

248248
error[E0716]: temporary value dropped while borrowed
249-
--> $DIR/promote-not.rs:69:31
249+
--> $DIR/promote-not.rs:70:31
250250
|
251251
LL | let _val: &'static _ = &[&TEST_DROP; 1];
252252
| ---------- ^^^^^^^^^ - temporary value is freed at the end of this statement
253253
| | |
254254
| | creates a temporary value which is freed while still in use
255255
| type annotation requires that borrow lasts for `'static`
256256

257-
error: aborting due to 24 previous errors
257+
error[E0716]: temporary value dropped while borrowed
258+
--> $DIR/promote-not.rs:79:26
259+
|
260+
LL | let x: &'static _ = &UnionWithCell { f1: 0 };
261+
| ---------- ^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
262+
| |
263+
| type annotation requires that borrow lasts for `'static`
264+
LL |
265+
LL | }
266+
| - temporary value is freed at the end of this statement
267+
268+
error: aborting due to 25 previous errors
258269

259270
For more information about this error, try `rustc --explain E0716`.

0 commit comments

Comments
 (0)