Skip to content

Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking #121893

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 11, 2024
Merged

Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking #121893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9a2886
add comment and test: we do not do value-based reasoning for promotio…
RalfJung Feb 29, 2024
279465b
const-checking: add some corner case tests, and fix some nits
RalfJung Mar 2, 2024
fb802f2
promote-not: add test that distinguishes promotion from outer scope rule
RalfJung Mar 11, 2024
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
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,10 @@ pub fn const_validate_mplace<'mir, 'tcx>(
_ if cid.promoted.is_some() => CtfeValidationMode::Promoted,
Some(mutbl) => CtfeValidationMode::Static { mutbl }, // a `static`
None => {
// In normal `const` (not promoted), the outermost allocation is always only copied,
// so having `UnsafeCell` in there is okay despite them being in immutable memory.
let allow_immutable_unsafe_cell = cid.promoted.is_none() && !inner;
CtfeValidationMode::Const { allow_immutable_unsafe_cell }
// This is a normal `const` (not promoted).
// The outermost allocation is always only copied, so having `UnsafeCell` in there
// is okay despite them being in immutable memory.
CtfeValidationMode::Const { allow_immutable_unsafe_cell: !inner }
}
};
ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/transform/check_consts/check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type QualifResults<'mir, 'tcx, Q> =
rustc_mir_dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'mir, 'mir, 'tcx, Q>>;

#[derive(Default)]
pub struct Qualifs<'mir, 'tcx> {
pub(crate) struct Qualifs<'mir, 'tcx> {
has_mut_interior: Option<QualifResults<'mir, 'tcx, HasMutInterior>>,
needs_drop: Option<QualifResults<'mir, 'tcx, NeedsDrop>>,
needs_non_const_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ where
if Q::in_adt_inherently(cx, def, args) {
return true;
}
// Don't do any value-based reasoning for unions.
if def.is_union() && Q::in_any_value_of_ty(cx, rvalue.ty(cx.body, cx.tcx)) {
return true;
}
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/consts/enclosing-scope-rule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//@build-pass
// Some code that looks like it might be relying on promotion, but actually this is using the
// enclosing-scope rule, meaning the reference is "extended" to outlive its block and live as long
// as the surrounding block (which in this case is the entire program). There are multiple
// allocations being interned at once.

struct Gen<T>(T);
impl<'a, T> Gen<&'a T> {
// Can't be promoted because `T` might not be `'static`.
const C: &'a [T] = &[];
}

// Can't be promoted because of `Drop`.
const V: &Vec<i32> = &Vec::new();

fn main() {}
21 changes: 20 additions & 1 deletion tests/ui/consts/promote-not.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![allow(unconditional_panic)]

use std::cell::Cell;
use std::mem::ManuallyDrop;

// We do not promote mutable references.
static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]); //~ ERROR temporary value dropped while borrowed
Expand Down Expand Up @@ -39,7 +40,16 @@ const TEST_INTERIOR_MUT: () = {
let _val: &'static _ = &(Cell::new(1), 2).1; //~ ERROR temporary value dropped while borrowed
};

const TEST_DROP: String = String::new();
// This gets accepted by the "outer scope" rule, not promotion.
const TEST_DROP_OUTER_SCOPE: &String = &String::new();
// To demonstrate that, we can rewrite it as follows. If this was promotion it would still work.
const TEST_DROP_NOT_PROMOTE: &String = {
let x = &String::new(); //~ ERROR destructor of `String` cannot be evaluated at compile-time
// The "dropped while borrowed" error seems to be suppressed, but the key point is that this
// fails to compile.
x
};


fn main() {
// We must not promote things with interior mutability. Not even if we "project it away".
Expand All @@ -58,6 +68,7 @@ fn main() {
let _val: &'static _ = &([1,2,3][4]+1); //~ ERROR temporary value dropped while borrowed

// No promotion of temporaries that need to be dropped.
const TEST_DROP: String = String::new();
let _val: &'static _ = &TEST_DROP;
//~^ ERROR temporary value dropped while borrowed
let _val: &'static _ = &&TEST_DROP;
Expand All @@ -69,4 +80,12 @@ fn main() {
let _val: &'static _ = &[&TEST_DROP; 1];
//~^ ERROR temporary value dropped while borrowed
//~| ERROR temporary value dropped while borrowed

// Make sure there is no value-based reasoning for unions.
union UnionWithCell {
f1: i32,
f2: ManuallyDrop<Cell<i32>>,
}
let x: &'static _ = &UnionWithCell { f1: 0 };
//~^ ERROR temporary value dropped while borrowed
}
73 changes: 47 additions & 26 deletions tests/ui/consts/promote-not.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0716]: temporary value dropped while borrowed
--> $DIR/promote-not.rs:8:50
--> $DIR/promote-not.rs:9:50
|
LL | static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]);
| ----------^^^^^^^^^-
Expand All @@ -9,7 +9,7 @@ LL | static mut TEST1: Option<&mut [i32]> = Some(&mut [1, 2, 3]);
| using this value as a static requires that borrow lasts for `'static`

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

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

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

error[E0493]: destructor of `String` cannot be evaluated at compile-time
--> $DIR/promote-not.rs:47:14
|
LL | let x = &String::new();
| ^^^^^^^^^^^^^ the destructor for this type cannot be evaluated in constants
...
LL | };
| - value is dropped here

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

error[E0716]: temporary value dropped while borrowed
--> $DIR/promote-not.rs:69:29
--> $DIR/promote-not.rs:80:29
|
LL | let _val: &'static _ = &[&TEST_DROP; 1];
| ---------- ^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
Expand All @@ -246,14 +255,26 @@ LL | }
| - temporary value is freed at the end of this statement

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

error: aborting due to 24 previous errors
error[E0716]: temporary value dropped while borrowed
--> $DIR/promote-not.rs:89:26
|
LL | let x: &'static _ = &UnionWithCell { f1: 0 };
| ---------- ^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
LL |
LL | }
| - temporary value is freed at the end of this statement

error: aborting due to 26 previous errors

For more information about this error, try `rustc --explain E0716`.
Some errors have detailed explanations: E0493, E0716.
For more information about an error, try `rustc --explain E0493`.
23 changes: 23 additions & 0 deletions tests/ui/consts/refs-to-cell-in-final.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,27 @@ static RAW_SYNC_S: SyncPtr<Cell<i32>> = SyncPtr { x: &Cell::new(42) };
const RAW_SYNC_C: SyncPtr<Cell<i32>> = SyncPtr { x: &Cell::new(42) };
//~^ ERROR: cannot refer to interior mutable data

// This one does not get promoted because of `Drop`, and then enters interesting codepaths because
// as a value it has no interior mutability, but as a type it does. See
// <https://github.com/rust-lang/rust/issues/121610>. Value-based reasoning for interior mutability
// is questionable (https://github.com/rust-lang/unsafe-code-guidelines/issues/493) so for now we
// reject this, though not with a great error message.
pub enum JsValue {
Undefined,
Object(Cell<bool>),
}
impl Drop for JsValue {
fn drop(&mut self) {}
}
const UNDEFINED: &JsValue = &JsValue::Undefined;
//~^ERROR: mutable pointer in final value of constant

// In contrast, this one works since it is being promoted.
const NONE: &'static Option<Cell<i32>> = &None;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe show off the promotion somewhere other than the trailing expression?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I didn't even realize that's possible -- I will add

// Making it clear that this is promotion, not "outer scope".
const NONE_EXPLICIT_PROMOTED: &'static Option<Cell<i32>> = {
    let x = &None;
    x
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, the behavior of this example changed between stable and nightly. On nightly it just says "can't drop String in const", on stable it additionally says "temporary value dropped while borrowed". I hope that's just suppressing two errors from the same location and not secretly actually doing promotion...

// Making it clear that this is promotion, not "outer scope".
const NONE_EXPLICIT_PROMOTED: &'static Option<Cell<i32>> = {
let x = &None;
x
};

fn main() {}
8 changes: 7 additions & 1 deletion tests/ui/consts/refs-to-cell-in-final.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ error[E0492]: constants cannot refer to interior mutable data
LL | const RAW_SYNC_C: SyncPtr<Cell<i32>> = SyncPtr { x: &Cell::new(42) };
| ^^^^^^^^^^^^^^ this borrow of an interior mutable value may end up in the final value

error: aborting due to 2 previous errors
error: encountered mutable pointer in final value of constant
--> $DIR/refs-to-cell-in-final.rs:30:1
|
LL | const UNDEFINED: &JsValue = &JsValue::Undefined;
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0492`.
Loading