Auto merge of #56070 - oli-obk:const_let, r=eddyb

Allow assignments in const contexts

fixes #54098
fixes #51251
fixes #52613

Author: bors

src/librustc/ich/impls_ty.rs

@@ -429,6 +429,7 @@ impl_stable_hash_for!(
         CalledClosureAsFunction,
         VtableForArgumentlessMethod,
         ModifiedConstantMemory,
+        ModifiedStatic,
         AssumptionNotHeld,
         InlineAsm,
         ReallocateNonBasePtr,

src/librustc/mir/interpret/error.rs

@@ -308,6 +308,7 @@ pub enum EvalErrorKind<'tcx, O> {
     CalledClosureAsFunction,
     VtableForArgumentlessMethod,
     ModifiedConstantMemory,
+    ModifiedStatic,
     AssumptionNotHeld,
     InlineAsm,
     TypeNotPrimitive(Ty<'tcx>),
@@ -412,6 +413,8 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
                 "tried to call a vtable function without arguments",
             ModifiedConstantMemory =>
                 "tried to modify constant memory",
+            ModifiedStatic =>
+                "tried to modify a static's initial value from another static's initializer",
             AssumptionNotHeld =>
                 "`assume` argument was false",
             InlineAsm =>

src/librustc_mir/interpret/machine.rs

@@ -89,7 +89,8 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
         Default +
         Clone;
 
-    /// The memory kind to use for copied statics -- or None if those are not supported.
+    /// The memory kind to use for copied statics -- or None if statics should not be mutated
+    /// and thus any such attempt will cause a `ModifiedStatic` error to be raised.
     /// Statics are copied under two circumstances: When they are mutated, and when
     /// `static_with_default_tag` or `find_foreign_static` (see below) returns an owned allocation
     /// that is added to the memory so that the work is not done twice.

src/librustc_mir/interpret/memory.rs

@@ -397,10 +397,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
             if alloc.mutability == Mutability::Immutable {
                 return err!(ModifiedConstantMemory);
             }
-            let kind = M::STATIC_KIND.expect(
-                "An allocation is being mutated but the machine does not expect that to happen"
-            );
-            Ok((MemoryKind::Machine(kind), alloc.into_owned()))
+            match M::STATIC_KIND {
+                Some(kind) => Ok((MemoryKind::Machine(kind), alloc.into_owned())),
+                None => err!(ModifiedStatic),
+            }
         });
         // Unpack the error type manually because type inference doesn't
         // work otherwise (and we cannot help it because `impl Trait`)

src/librustc_mir/transform/const_prop.rs

@@ -199,6 +199,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
                     | CalledClosureAsFunction
                     | VtableForArgumentlessMethod
                     | ModifiedConstantMemory
+                    | ModifiedStatic
                     | AssumptionNotHeld
                     // FIXME: should probably be removed and turned into a bug! call
                     | TypeNotPrimitive(_)

src/librustc_mir/transform/qualify_consts.rs

@@ -243,13 +243,52 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
             return;
         }
 
-        match *dest {
-            Place::Local(index) if (self.mir.local_kind(index) == LocalKind::Var ||
-                                   self.mir.local_kind(index) == LocalKind::Arg) &&
-                                   self.tcx.sess.features_untracked().const_let => {
-                debug!("store to var {:?}", index);
-                self.local_qualif[index] = Some(self.qualif);
+        if self.tcx.features().const_let {
+            let mut dest = dest;
+            let index = loop {
+                match dest {
+                    // with `const_let` active, we treat all locals equal
+                    Place::Local(index) => break *index,
+                    // projections are transparent for assignments
+                    // we qualify the entire destination at once, even if just a field would have
+                    // stricter qualification
+                    Place::Projection(proj) => {
+                        // Catch more errors in the destination. `visit_place` also checks various
+                        // projection rules like union field access and raw pointer deref
+                        self.visit_place(
+                            dest,
+                            PlaceContext::MutatingUse(MutatingUseContext::Store),
+                            location
+                        );
+                        dest = &proj.base;
+                    },
+                    Place::Promoted(..) => bug!("promoteds don't exist yet during promotion"),
+                    Place::Static(..) => {
+                        // Catch more errors in the destination. `visit_place` also checks that we
+                        // do not try to access statics from constants or try to mutate statics
+                        self.visit_place(
+                            dest,
+                            PlaceContext::MutatingUse(MutatingUseContext::Store),
+                            location
+                        );
+                        return;
+                    }
+                }
+            };
+            debug!("store to var {:?}", index);
+            match &mut self.local_qualif[index] {
+                // this is overly restrictive, because even full assignments do not clear the qualif
+                // While we could special case full assignments, this would be inconsistent with
+                // aggregates where we overwrite all fields via assignments, which would not get
+                // that feature.
+                Some(ref mut qualif) => *qualif = *qualif | self.qualif,
+                // insert new qualification
+                qualif @ None => *qualif = Some(self.qualif),
             }
+            return;
+        }
+
+        match *dest {
             Place::Local(index) if self.mir.local_kind(index) == LocalKind::Temp ||
                                    self.mir.local_kind(index) == LocalKind::ReturnPointer => {
                 debug!("store to {:?} (temp or return pointer)", index);
@@ -478,6 +517,16 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
 
                 // Only allow statics (not consts) to refer to other statics.
                 if self.mode == Mode::Static || self.mode == Mode::StaticMut {
+                    if context.is_mutating_use() {
+                        // this is not strictly necessary as miri will also bail out
+                        // For interior mutability we can't really catch this statically as that
+                        // goes through raw pointers and intermediate temporaries, so miri has
+                        // to catch this anyway
+                        self.tcx.sess.span_err(
+                            self.span,
+                            "cannot mutate statics in the initializer of another static",
+                        );
+                    }
                     return;
                 }
                 self.add(Qualif::NOT_CONST);

src/test/ui/consts/const-eval/assign-to-static-within-other-static-2.rs

@@ -0,0 +1,30 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// New test for #53818: modifying static memory at compile-time is not allowed.
+// The test should never compile successfully
+
+#![feature(const_raw_ptr_deref)]
+#![feature(const_let)]
+
+use std::cell::UnsafeCell;
+
+struct Foo(UnsafeCell<u32>);
+
+unsafe impl Send for Foo {}
+unsafe impl Sync for Foo {}
+
+static FOO: Foo = Foo(UnsafeCell::new(42));
+
+static BAR: () = unsafe {
+    *FOO.0.get() = 5; //~ ERROR could not evaluate static initializer
+};
+
+fn main() {}

src/test/ui/consts/const-eval/assign-to-static-within-other-static-2.stderr

@@ -0,0 +1,9 @@
+error[E0080]: could not evaluate static initializer
+  --> $DIR/assign-to-static-within-other-static-2.rs:27:5
+   |
+LL |     *FOO.0.get() = 5; //~ ERROR could not evaluate static initializer
+   |     ^^^^^^^^^^^^^^^^ tried to modify a static's initial value from another static's initializer
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0080`.

src/test/ui/consts/const-eval/assign-to-static-within-other-static.rs

@@ -0,0 +1,24 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// New test for #53818: modifying static memory at compile-time is not allowed.
+// The test should never compile successfully
+
+#![feature(const_raw_ptr_deref)]
+#![feature(const_let)]
+
+use std::cell::UnsafeCell;
+
+static mut FOO: u32 = 42;
+static BOO: () = unsafe {
+    FOO = 5; //~ ERROR cannot mutate statics in the initializer of another static
+};
+
+fn main() {}

src/test/ui/consts/const-eval/assign-to-static-within-other-static.stderr

@@ -0,0 +1,8 @@
+error: cannot mutate statics in the initializer of another static
+  --> $DIR/assign-to-static-within-other-static.rs:21:5
+   |
+LL |     FOO = 5; //~ ERROR cannot mutate statics in the initializer of another static
+   |     ^^^^^^^
+
+error: aborting due to previous error
+