Skip to content
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

Jump to bottom

remove this weird special case from promotion #78363

Merged
merged 3 commits into from
Dec 9, 2020
Merged

remove this weird special case from promotion #78363

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
bd837e8
make sure we do not promote things with interior mutability
RalfJung Oct 25, 2020
99a44ed
remove a hack that seems to only benefit a few very special cases
RalfJung Oct 25, 2020
d057a93
remove a test that does not work any more
RalfJung Nov 15, 2020
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
58 changes: 6 additions & 52 deletions compiler/rustc_mir/src/transform/promote_consts.rs
View file
Open in desktop
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 {
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
return Err(Unpromotable);
}
if self.qualif_local::<qualifs::NeedsDrop>(place.local) {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
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 {
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
return Err(Unpromotable);
}
Expand Down
14 changes: 13 additions & 1 deletion src/test/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,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
Copy link
Member Author

Choose a reason for hiding this comment

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

These two already failed before.

}
43 changes: 37 additions & 6 deletions src/test/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: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
View file
Open in desktop

This file was deleted.