Skip to content

Commit

Permalink
Auto merge of #74945 - dingxiangfei2009:promote-static-ref-deref, r=o…
Browse files Browse the repository at this point in the history
…li-obk

[mir] Special treatment for dereferencing a borrow to a static definition

Fix #70584.

As suggested by @oli-obk in this [comment](#70584 (comment)), one can chase the definition of the local variable being de-referenced and check if it is a true static variable. If that is the case, `validate_place` will admit the promotion.

This is my first time to contribute to `rustc`, and I have two questions.
1. A generalization to some extent is applied to decide if the promotion is possible in the static context. In case that there are more projection operations preceding the de-referencing, `validate_place` recursively decent into inner projection operations. I have put thoughts into its correctness but I am not totally sure about it.
2. I have a hard time to find a good place for the test case. This patch has to do with MIR, but this test case would look out of place compared to other tests in `src/test/ui/mir` or `src/test/ui/borrowck` because it does not generate errors while others do. It is tentatively placed in `src/test/ui/statics` for now.

Thank you for any comments and suggestions!
  • Loading branch information
bors committed Aug 1, 2020
2 parents 22e6099 + c511454 commit 18e2a89
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 2 deletions.
42 changes: 40 additions & 2 deletions src/librustc_mir/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,9 +502,47 @@ impl<'tcx> Validator<'_, 'tcx> {
fn validate_place(&self, place: PlaceRef<'tcx>) -> Result<(), Unpromotable> {
match place {
PlaceRef { local, projection: [] } => self.validate_local(local),
PlaceRef { local: _, projection: [proj_base @ .., elem] } => {
PlaceRef { local, projection: [proj_base @ .., elem] } => {
match *elem {
ProjectionElem::Deref | ProjectionElem::Downcast(..) => {
ProjectionElem::Deref => {
let mut not_promotable = true;
// This is a special treatment for cases like *&STATIC where STATIC is a
// global static variable.
// This pattern is generated only when global static variables are directly
// accessed and is qualified for promotion safely.
if let TempState::Defined { location, .. } = self.temps[local] {
let def_stmt =
self.body[location.block].statements.get(location.statement_index);
if let Some(Statement {
kind:
StatementKind::Assign(box (_, Rvalue::Use(Operand::Constant(c)))),
..
}) = def_stmt
{
if let Some(did) = c.check_static_ptr(self.tcx) {
if let Some(hir::ConstContext::Static(..)) = self.const_kind {
// The `is_empty` predicate is introduced to exclude the case
// where the projection operations are [ .field, * ].
// The reason is because promotion will be illegal if field
// accesses preceed the dereferencing.
// Discussion can be found at
// https://github.com/rust-lang/rust/pull/74945#discussion_r463063247
// There may be opportunity for generalization, but this needs to be
// accounted for.
if proj_base.is_empty()
&& !self.tcx.is_thread_local_static(did)
{
not_promotable = false;
}
}
}
}
}
if not_promotable {
return Err(Unpromotable);
}
}
ProjectionElem::Downcast(..) => {
return Err(Unpromotable);
}

Expand Down
34 changes: 34 additions & 0 deletions src/test/ui/statics/static-promotion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// check-pass

// Use of global static variables in literal values should be allowed for
// promotion.
// This test is to demonstrate the issue raised in
// https://github.com/rust-lang/rust/issues/70584

// Literal values were previously promoted into local static values when
// other global static variables are used.

struct A<T: 'static>(&'static T);
struct B<T: 'static + ?Sized> {
x: &'static T,
}
static STR: &'static [u8] = b"hi";
static C: A<B<B<[u8]>>> = {
A(&B {
x: &B { x: STR },
})
};

pub struct Slice(&'static [i32]);

static CONTENT: i32 = 42;
pub static CONTENT_MAP: Slice = Slice(&[CONTENT]);

pub static FOO: (i32, i32) = (42, 43);
pub static CONTENT_MAP2: Slice = Slice(&[FOO.0]);

fn main() {
assert_eq!(b"hi", C.0.x.x);
assert_eq!(&[42], CONTENT_MAP.0);
assert_eq!(&[42], CONTENT_MAP2.0);
}

0 comments on commit 18e2a89

Please sign in to comment.