summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-12-09 09:13:54 +0000
committerbors <bors@rust-lang.org>2020-12-09 09:13:54 +0000
commitc0bfe3485f97f267cc8adec724f109c56dab5526 (patch)
tree110ddab74de64d4acd4a0d416a6f4b0734a508ab
parentdb85512bd8d922e0785029eff41152b0f35690f1 (diff)
parentd057a93e6f43eb410b9c348a8fe590bdc4452ae6 (diff)
downloadrust-c0bfe3485f97f267cc8adec724f109c56dab5526.tar.gz
Auto merge of #78363 - RalfJung:promotion, r=oli-obk
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 https://github.com/rust-lang/rust/issues/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`
-rw-r--r--compiler/rustc_mir/src/transform/promote_consts.rs58
-rw-r--r--src/test/ui/consts/promote-not.rs14
-rw-r--r--src/test/ui/consts/promote-not.stderr43
-rw-r--r--src/test/ui/issues/issue-49955-2.rs19
4 files changed, 56 insertions, 78 deletions
diff --git a/compiler/rustc_mir/src/transform/promote_consts.rs b/compiler/rustc_mir/src/transform/promote_consts.rs
index 927aae82a36..8d5ed747c3f 100644
--- a/compiler/rustc_mir/src/transform/promote_consts.rs
+++ b/compiler/rustc_mir/src/transform/promote_consts.rs
@@ -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;
@@ -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;
@@ -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);
}
diff --git a/src/test/ui/consts/promote-not.rs b/src/test/ui/consts/promote-not.rs
index 30bb9917bf7..1e4d8586b87 100644
--- a/src/test/ui/consts/promote-not.rs
+++ b/src/test/ui/consts/promote-not.rs
@@ -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
@@ -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
+}
diff --git a/src/test/ui/consts/promote-not.stderr b/src/test/ui/consts/promote-not.stderr
index 6ca7a4c273e..6e76d9ee6c1 100644
--- a/src/test/ui/consts/promote-not.stderr
+++ b/src/test/ui/consts/promote-not.stderr
@@ -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]);
| ----------^^^^^^^^^-
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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`.
diff --git a/src/test/ui/issues/issue-49955-2.rs b/src/test/ui/issues/issue-49955-2.rs
deleted file mode 100644
index 267ed746322..00000000000
--- a/src/test/ui/issues/issue-49955-2.rs
+++ /dev/null
@@ -1,19 +0,0 @@
-// run-pass
-// compile-flags: -Z borrowck=mir
-
-use std::cell::Cell;
-
-const FIVE: Cell<i32> = Cell::new(5);
-
-#[inline(never)]
-fn tuple_field() -> &'static u32 {
- // This test is MIR-borrowck-only because the old borrowck
- // doesn't agree that borrows of "frozen" (i.e., without any
- // interior mutability) fields of non-frozen temporaries,
- // should be promoted, while MIR promotion does promote them.
- &(FIVE, 42).1
-}
-
-fn main() {
- assert_eq!(tuple_field().to_string(), "42");
-}