Skip to content

Commit

Permalink
Auto merge of #78363 - RalfJung:promotion, r=oli-obk
Browse files Browse the repository at this point in the history
remove this weird special case from promotion

Promotion has a special case to ignore interior mutability under some specific circumstances. The purpose of this PR is to figure out what changes if we remove that. Since `Cell::new` and friends only get promoted inside `const`/`static` initializers these days, it actually is not easy to exploit this case: you need something like
```rust
const TEST_INTERIOR_MUT: () = {
    // The "0." case is already ruled out by not permitting any interior mutability in `const`.
    let _val: &'static _ = &(Cell::new(1), 2).1;
};
```

I assume something like `&Some(&(Cell::new(1), 2).1)` would hit the nested case inside `validate_rvalue`... though I am not sure why that would not just trigger nested promotion, first promoting the inner reference and then the outer one?

Fixes #67534 (by simply rejecting that code^^)

r? `@oli-obk` (but for now this is not meant to be merged!)
Cc `@rust-lang/wg-const-eval`
  • Loading branch information
bors committed Dec 9, 2020
2 parents db85512 + d057a93 commit c0bfe34
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 78 deletions.
58 changes: 6 additions & 52 deletions compiler/rustc_mir/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc_middle::ty::cast::CastTy;
use rustc_middle::ty::subst::InternalSubsts;
use rustc_middle::ty::{self, List, TyCtxt, TypeFoldable};
use rustc_span::symbol::sym;
use rustc_span::{Span, DUMMY_SP};
use rustc_span::Span;

use rustc_index::vec::{Idx, IndexVec};
use rustc_target::spec::abi::Abi;
Expand Down Expand Up @@ -326,41 +326,16 @@ impl<'tcx> Validator<'_, 'tcx> {
if place.projection.contains(&ProjectionElem::Deref) {
return Err(Unpromotable);
}

let mut has_mut_interior =
self.qualif_local::<qualifs::HasMutInterior>(place.local);
// HACK(eddyb) this should compute the same thing as
// `<HasMutInterior as Qualif>::in_projection` from
// `check_consts::qualifs` but without recursion.
if has_mut_interior {
// This allows borrowing fields which don't have
// `HasMutInterior`, from a type that does, e.g.:
// `let _: &'static _ = &(Cell::new(1), 2).1;`
let mut place_projection = &place.projection[..];
// FIXME(eddyb) use a forward loop instead of a reverse one.
while let &[ref proj_base @ .., elem] = place_projection {
// FIXME(eddyb) this is probably excessive, with
// the exception of `union` member accesses.
let ty =
Place::ty_from(place.local, proj_base, self.body, self.tcx)
.projection_ty(self.tcx, elem)
.ty;
if ty.is_freeze(self.tcx.at(DUMMY_SP), self.param_env) {
has_mut_interior = false;
break;
}

place_projection = proj_base;
}
if self.qualif_local::<qualifs::NeedsDrop>(place.local) {
return Err(Unpromotable);
}

// FIXME(eddyb) this duplicates part of `validate_rvalue`.
let has_mut_interior =
self.qualif_local::<qualifs::HasMutInterior>(place.local);
if has_mut_interior {
return Err(Unpromotable);
}
if self.qualif_local::<qualifs::NeedsDrop>(place.local) {
return Err(Unpromotable);
}

if let BorrowKind::Mut { .. } = kind {
let ty = place.ty(self.body, self.tcx).ty;
Expand Down Expand Up @@ -692,28 +667,7 @@ impl<'tcx> Validator<'_, 'tcx> {

self.validate_place(place)?;

// HACK(eddyb) this should compute the same thing as
// `<HasMutInterior as Qualif>::in_projection` from
// `check_consts::qualifs` but without recursion.
let mut has_mut_interior =
self.qualif_local::<qualifs::HasMutInterior>(place.local);
if has_mut_interior {
let mut place_projection = place.projection;
// FIXME(eddyb) use a forward loop instead of a reverse one.
while let &[ref proj_base @ .., elem] = place_projection {
// FIXME(eddyb) this is probably excessive, with
// the exception of `union` member accesses.
let ty = Place::ty_from(place.local, proj_base, self.body, self.tcx)
.projection_ty(self.tcx, elem)
.ty;
if ty.is_freeze(self.tcx.at(DUMMY_SP), self.param_env) {
has_mut_interior = false;
break;
}

place_projection = proj_base;
}
}
let has_mut_interior = self.qualif_local::<qualifs::HasMutInterior>(place.local);
if has_mut_interior {
return Err(Unpromotable);
}
Expand Down
14 changes: 13 additions & 1 deletion src/test/ui/consts/promote-not.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#![allow(unconditional_panic, const_err)]
#![feature(const_fn, const_fn_union)]

use std::cell::Cell;

// 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 @@ -32,4 +34,14 @@ const TEST_UNION: () = {
let _x: &'static i32 = &unsafe { U { x: 0 }.x }; //~ ERROR temporary value dropped while borrowed
};

fn main() {}
// In a `const`, we do not promote things with interior mutability. Not even if we "project it away".
const TEST_INTERIOR_MUT: () = {
// The "0." case is already ruled out by not permitting any interior mutability in `const`.
let _val: &'static _ = &(Cell::new(1), 2).1; //~ ERROR temporary value dropped while borrowed
};

fn main() {
// We must not promote things with interior mutability. Not even if we "project it away".
let _val: &'static _ = &(Cell::new(1), 2).0; //~ ERROR temporary value dropped while borrowed
let _val: &'static _ = &(Cell::new(1), 2).1; //~ ERROR temporary value dropped while borrowed
}
43 changes: 37 additions & 6 deletions src/test/ui/consts/promote-not.stderr
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:7: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:10:18
--> $DIR/promote-not.rs:12:18
|
LL | let x = &mut [1,2,3];
| ^^^^^^^ creates a temporary 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:19:32
--> $DIR/promote-not.rs:21:32
|
LL | let _x: &'static () = &foo();
| ----------- ^^^^^ creates a temporary 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:27:29
--> $DIR/promote-not.rs:29:29
|
LL | let _x: &'static i32 = &unsafe { U { x: 0 }.x };
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
Expand All @@ -39,7 +39,7 @@ LL | }
| - temporary value is freed at the end of this statement

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

error: aborting due to 5 previous errors
error[E0716]: temporary value dropped while borrowed
--> $DIR/promote-not.rs:40:29
|
LL | let _val: &'static _ = &(Cell::new(1), 2).1;
| ---------- ^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
LL | };
| - temporary value is freed at the end of this statement

error[E0716]: temporary value dropped while borrowed
--> $DIR/promote-not.rs:45:29
|
LL | let _val: &'static _ = &(Cell::new(1), 2).0;
| ---------- ^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
LL | let _val: &'static _ = &(Cell::new(1), 2).1;
LL | }
| - temporary value is freed at the end of this statement

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

error: aborting due to 8 previous errors

For more information about this error, try `rustc --explain E0716`.
19 changes: 0 additions & 19 deletions src/test/ui/issues/issue-49955-2.rs

This file was deleted.

0 comments on commit c0bfe34

Please sign in to comment.