Auto merge of rust-lang#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

Author: bors

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(()),