about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-08-01 04:34:05 +0000
committerbors <bors@rust-lang.org>2020-08-01 04:34:05 +0000
commit18e2a891999e66d856ea13db879e93076de9e237 (patch)
tree9c0d326940607e173861961b77c0b49378a13cd9
parent22e6099330cde0e7b1529774fe27874f8326de7a (diff)
parentc5114549d74f6092517af6ea630ec5a26317ae93 (diff)
downloadrust-18e2a891999e66d856ea13db879e93076de9e237.tar.gz
rust-18e2a891999e66d856ea13db879e93076de9e237.zip
Auto merge of #74945 - dingxiangfei2009:promote-static-ref-deref, r=oli-obk
[mir] Special treatment for dereferencing a borrow to a static definition

Fix #70584.

As suggested by @oli-obk in this [comment](https://github.com/rust-lang/rust/issues/70584#issuecomment-626009260), 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!
-rw-r--r--src/librustc_mir/transform/promote_consts.rs42
-rw-r--r--src/test/ui/statics/static-promotion.rs34
2 files changed, 74 insertions, 2 deletions
diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs
index 59a8415ef96..f6847348063 100644
--- a/src/librustc_mir/transform/promote_consts.rs
+++ b/src/librustc_mir/transform/promote_consts.rs
@@ -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);
                     }
 
diff --git a/src/test/ui/statics/static-promotion.rs b/src/test/ui/statics/static-promotion.rs
new file mode 100644
index 00000000000..bd8910bdb3f
--- /dev/null
+++ b/src/test/ui/statics/static-promotion.rs
@@ -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);
+}