Auto merge of #56160 - oli-obk:const_fn_let, r=nikomatsakis

Fix various aspects around `let` bindings inside const functions

* forbid `let` bindings in const contexts that use short circuiting operators
* harden analysis code against derefs of mutable references

Initially this PR was about stabilizing `let` bindings, but too many flaws were exposed that need some more testing on nightly

src/libcore/lib.rs

@@ -121,7 +121,6 @@
 #![feature(const_slice_len)]
 #![feature(const_str_as_bytes)]
 #![feature(const_str_len)]
-#![feature(const_let)]
 #![feature(const_int_rotate)]
 #![feature(const_int_wrapping)]
 #![feature(const_int_sign)]

src/librustc/mir/mod.rs

@@ -149,6 +149,14 @@ pub struct Mir<'tcx> {
     /// This is used for the "rust-call" ABI.
     pub spread_arg: Option<Local>,
 
+    /// Mark this MIR of a const context other than const functions as having converted a `&&` or
+    /// `||` expression into `&` or `|` respectively. This is problematic because if we ever stop
+    /// this conversion from happening and use short circuiting, we will cause the following code
+    /// to change the value of `x`: `let mut x = 42; false && { x = 55; true };`
+    ///
+    /// List of places where control flow was destroyed. Used for error reporting.
+    pub control_flow_destroyed: Vec<(Span, String)>,
+
     /// A span representing this MIR, for error reporting
     pub span: Span,
 
@@ -167,6 +175,7 @@ impl<'tcx> Mir<'tcx> {
         arg_count: usize,
         upvar_decls: Vec<UpvarDecl>,
         span: Span,
+        control_flow_destroyed: Vec<(Span, String)>,
     ) -> Self {
         // We need `arg_count` locals, and one for the return place
         assert!(
@@ -191,6 +200,7 @@ impl<'tcx> Mir<'tcx> {
             spread_arg: None,
             span,
             cache: cache::Cache::new(),
+            control_flow_destroyed,
         }
     }
 
@@ -421,6 +431,7 @@ impl_stable_hash_for!(struct Mir<'tcx> {
     arg_count,
     upvar_decls,
     spread_arg,
+    control_flow_destroyed,
     span,
     cache
 });
@@ -1748,6 +1759,9 @@ pub enum StatementKind<'tcx> {
     /// (e.g., inspecting constants and discriminant values), and the
     /// kind of pattern it comes from. This is in order to adapt potential
     /// error messages to these specific patterns.
+    ///
+    /// Note that this also is emitted for regular `let` bindings to ensure that locals that are
+    /// never accessed still get some sanity checks for e.g. `let x: ! = ..;`
     FakeRead(FakeReadCause, Place<'tcx>),
 
     /// Write the discriminant for a variant to the enum Place.
@@ -2984,6 +2998,7 @@ BraceStructTypeFoldableImpl! {
         arg_count,
         upvar_decls,
         spread_arg,
+        control_flow_destroyed,
         span,
         cache,
     }

src/librustc/ty/structural_impls.rs

@@ -35,6 +35,7 @@ CloneTypeFoldableAndLiftImpls! {
     usize,
     ::ty::layout::VariantIdx,
     u64,
+    String,
     ::middle::region::Scope,
     ::syntax::ast::FloatTy,
     ::syntax::ast::NodeId,

src/librustc_mir/build/mod.rs

@@ -849,15 +849,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             }
         }
 
-        Mir::new(self.cfg.basic_blocks,
-                 self.source_scopes,
-                 ClearCrossCrate::Set(self.source_scope_local_data),
-                 IndexVec::new(),
-                 yield_ty,
-                 self.local_decls,
-                 self.arg_count,
-                 self.upvar_decls,
-                 self.fn_span
+        Mir::new(
+            self.cfg.basic_blocks,
+            self.source_scopes,
+            ClearCrossCrate::Set(self.source_scope_local_data),
+            IndexVec::new(),
+            yield_ty,
+            self.local_decls,
+            self.arg_count,
+            self.upvar_decls,
+            self.fn_span,
+            self.hir.control_flow_destroyed(),
         )
     }
 

src/librustc_mir/hair/cx/expr.rs

@@ -372,13 +372,21 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
                     // FIXME(eddyb) use logical ops in constants when
                     // they can handle that kind of control-flow.
                     (hir::BinOpKind::And, hir::Constness::Const) => {
+                        cx.control_flow_destroyed.push((
+                            op.span,
+                            "`&&` operator".into(),
+                        ));
                         ExprKind::Binary {
                             op: BinOp::BitAnd,
                             lhs: lhs.to_ref(),
                             rhs: rhs.to_ref(),
                         }
                     }
                     (hir::BinOpKind::Or, hir::Constness::Const) => {
+                        cx.control_flow_destroyed.push((
+                            op.span,
+                            "`||` operator".into(),
+                        ));
                         ExprKind::Binary {
                             op: BinOp::BitOr,
                             lhs: lhs.to_ref(),

src/librustc_mir/hair/cx/mod.rs

@@ -56,6 +56,9 @@ pub struct Cx<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
 
     /// True if this constant/function needs overflow checks.
     check_overflow: bool,
+
+    /// See field with the same name on `Mir`
+    control_flow_destroyed: Vec<(Span, String)>,
 }
 
 impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
@@ -96,9 +99,13 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
             constness,
             body_owner_kind,
             check_overflow,
+            control_flow_destroyed: Vec::new(),
         }
     }
 
+    pub fn control_flow_destroyed(self) -> Vec<(Span, String)> {
+        self.control_flow_destroyed
+    }
 }
 
 impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {

src/librustc_mir/shim.rs

@@ -219,7 +219,8 @@ fn build_drop_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         local_decls_for_sig(&sig, span),
         sig.inputs().len(),
         vec![],
-        span
+        span,
+        vec![],
     );
 
     if let Some(..) = ty {
@@ -396,7 +397,8 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> {
             self.local_decls,
             self.sig.inputs().len(),
             vec![],
-            self.span
+            self.span,
+            vec![],
         )
     }
 
@@ -844,7 +846,8 @@ fn build_call_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         local_decls,
         sig.inputs().len(),
         vec![],
-        span
+        span,
+        vec![],
     );
     if let Abi::RustCall = sig.abi {
         mir.spread_arg = Some(Local::new(sig.inputs().len()));
@@ -921,6 +924,7 @@ pub fn build_adt_ctor<'a, 'gcx, 'tcx>(infcx: &infer::InferCtxt<'a, 'gcx, 'tcx>,
         local_decls,
         sig.inputs().len(),
         vec![],
-        span
+        span,
+        vec![],
     )
 }

src/librustc_mir/transform/promote_consts.rs

@@ -412,7 +412,8 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
                 initial_locals,
                 0,
                 vec![],
-                mir.span
+                mir.span,
+                vec![],
             ),
             tcx,
             source: mir,

src/librustc_mir/transform/qualify_consts.rs

@@ -553,7 +553,13 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                     this.super_place(place, context, location);
                     match proj.elem {
                         ProjectionElem::Deref => {
-                            this.add(Qualif::NOT_CONST);
+                            if context.is_mutating_use() {
+                                // `not_const` errors out in const contexts
+                                this.not_const()
+                            } else {
+                                // just make sure this doesn't get promoted
+                                this.add(Qualif::NOT_CONST);
+                            }
                             let base_ty = proj.base.ty(this.mir, this.tcx).to_ty(this.tcx);
                             match this.mode {
                                 Mode::Fn => {},
@@ -1178,7 +1184,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
             if self.mir.local_kind(index) == LocalKind::Var &&
                self.const_fn_arg_vars.insert(index) &&
                !self.tcx.features().const_let {
-
                 // Direct use of an argument is permitted.
                 match *rvalue {
                     Rvalue::Use(Operand::Copy(Place::Local(local))) |
@@ -1189,7 +1194,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                     }
                     _ => {}
                 }
-
                 // Avoid a generic error for other uses of arguments.
                 if self.qualif.contains(Qualif::FN_ARGUMENT) {
                     let decl = &self.mir.local_decls[index];
@@ -1348,6 +1352,37 @@ impl MirPass for QualifyAndPromoteConstants {
             // Do the actual promotion, now that we know what's viable.
             promote_consts::promote_candidates(mir, tcx, temps, candidates);
         } else {
+            if !mir.control_flow_destroyed.is_empty() {
+                let mut locals = mir.vars_iter();
+                if let Some(local) = locals.next() {
+                    let span = mir.local_decls[local].source_info.span;
+                    let mut error = tcx.sess.struct_span_err(
+                        span,
+                        &format!(
+                            "new features like let bindings are not permitted in {}s \
+                            which also use short circuiting operators",
+                            mode,
+                        ),
+                    );
+                    for (span, kind) in mir.control_flow_destroyed.iter() {
+                        error.span_note(
+                            *span,
+                            &format!("use of {} here does not actually short circuit due to \
+                            the const evaluator presently not being able to do control flow. \
+                            See https://github.com/rust-lang/rust/issues/49146 for more \
+                            information.", kind),
+                        );
+                    }
+                    for local in locals {
+                        let span = mir.local_decls[local].source_info.span;
+                        error.span_note(
+                            span,
+                            "more locals defined here",
+                        );
+                    }
+                    error.emit();
+                }
+            }
             let promoted_temps = if mode == Mode::Const {
                 // Already computed by `mir_const_qualif`.
                 const_promoted_temps.unwrap()

src/librustc_mir/transform/qualify_min_const_fn.rs

@@ -230,7 +230,7 @@ fn check_statement(
             check_rvalue(tcx, mir, rval, span)
         }
 
-        StatementKind::FakeRead(..) => Err((span, "match in const fn is unstable".into())),
+        StatementKind::FakeRead(_, place) => check_place(tcx, mir, place, span, PlaceMode::Read),
 
         // just an assignment
         StatementKind::SetDiscriminant { .. } => Ok(()),

src/test/compile-fail/const-fn-error.rs

@@ -8,14 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![feature(const_fn)]
+#![feature(const_fn, const_let)]
 
 const X : usize = 2;
 
 const fn f(x: usize) -> usize {
     let mut sum = 0;
-    //~^ let bindings in constant functions are unstable
-    //~| statements in constant functions are unstable
     for i in 0..x {
         //~^ ERROR E0015
         //~| ERROR E0019

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

@@ -24,7 +24,7 @@ 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
+    *FOO.0.get() = 5; //~ ERROR contains unimplemented expression type
 };
 
 fn main() {}

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

@@ -1,9 +1,9 @@
-error[E0080]: could not evaluate static initializer
+error[E0019]: static contains unimplemented expression type
   --> $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
+LL |     *FOO.0.get() = 5; //~ ERROR contains unimplemented expression type
+   |     ^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0080`.
+For more information about this error, try `rustc --explain E0019`.

src/test/ui/consts/const-eval/mod-static-with-const-fn.rs

@@ -27,9 +27,7 @@ fn foo() {}
 
 static BAR: () = unsafe {
     *FOO.0.get() = 5;
-    // we do not error on the above access, because that is not detectable statically. Instead,
-    // const evaluation will error when trying to evaluate it. Due to the error below, we never even
-    // attempt to const evaluate `BAR`, so we don't see the error
+    //~^ contains unimplemented expression
 
     foo();
     //~^ ERROR calls in statics are limited to constant functions, tuple structs and tuple variants

src/test/ui/consts/const-eval/mod-static-with-const-fn.stderr

@@ -1,9 +1,16 @@
+error[E0019]: static contains unimplemented expression type
+  --> $DIR/mod-static-with-const-fn.rs:29:5
+   |
+LL |     *FOO.0.get() = 5;
+   |     ^^^^^^^^^^^^^^^^
+
 error[E0015]: calls in statics are limited to constant functions, tuple structs and tuple variants
-  --> $DIR/mod-static-with-const-fn.rs:34:5
+  --> $DIR/mod-static-with-const-fn.rs:32:5
    |
 LL |     foo();
    |     ^^^^^
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 
-For more information about this error, try `rustc --explain E0015`.
+Some errors occurred: E0015, E0019.
+For more information about an error, try `rustc --explain E0015`.

src/test/ui/consts/const-fn-not-safe-for-const.rs

@@ -37,15 +37,13 @@ const fn get_Y_addr() -> &'static u32 {
 }
 
 const fn get() -> u32 {
-    let x = 22;
-    //~^ ERROR let bindings in constant functions are unstable
-    //~| ERROR statements in constant functions are unstable
-    let y = 44;
-    //~^ ERROR let bindings in constant functions are unstable
-    //~| ERROR statements in constant functions are unstable
+    let x = 22; //~ ERROR let bindings in constant functions are unstable
+//~^ ERROR statements in constant functions
+    let y = 44; //~ ERROR let bindings in constant functions are unstable
+//~^ ERROR statements in constant functions
     x + y
-    //~^ ERROR let bindings in constant functions are unstable
-    //~| ERROR let bindings in constant functions are unstable
+//~^ ERROR let bindings in constant functions are unstable
+//~| ERROR let bindings in constant functions are unstable
 }
 
 fn main() {}