Reenable const_let feature gate

Author: oli-obk

src/librustc_mir/transform/qualify_consts.rs

@@ -30,7 +30,7 @@ use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUs
 use rustc::middle::lang_items;
 use rustc_target::spec::abi::Abi;
 use syntax::ast::LitKind;
-use syntax::feature_gate::{UnstableFeatures, emit_feature_err, GateIssue};
+use syntax::feature_gate::{UnstableFeatures, feature_err, emit_feature_err, GateIssue};
 use syntax_pos::{Span, DUMMY_SP};
 
 use std::fmt;
@@ -114,6 +114,7 @@ struct Qualifier<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     param_env: ty::ParamEnv<'tcx>,
     local_qualif: IndexVec<Local, Option<Qualif>>,
     qualif: Qualif,
+    const_fn_arg_vars: BitSet<Local>,
     temp_promotion_state: IndexVec<Local, TempState>,
     promotion_candidates: Vec<Candidate>
 }
@@ -148,6 +149,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
             param_env,
             local_qualif,
             qualif: Qualif::empty(),
+            const_fn_arg_vars: BitSet::new_empty(mir.local_decls.len()),
             temp_promotion_state: temps,
             promotion_candidates: vec![]
         }
@@ -176,6 +178,26 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
         }
     }
 
+    /// Error about extra statements in a constant.
+    fn statement_like(&mut self) {
+        self.add(Qualif::NOT_CONST);
+        if self.mode != Mode::Fn {
+            let mut err = feature_err(
+                &self.tcx.sess.parse_sess,
+                "const_let",
+                self.span,
+                GateIssue::Language,
+                &format!("statements in {}s are unstable", self.mode),
+            );
+            if self.tcx.sess.teach(&err.get_code().unwrap()) {
+                err.note("Blocks in constants may only contain items (such as constant, function \
+                          definition, etc...) and a tail expression.");
+                err.help("To avoid it, you have to replace the non-item object.");
+            }
+            err.emit();
+        }
+    }
+
     /// Add the given qualification to self.qualif.
     fn add(&mut self, qualif: Qualif) {
         self.qualif = self.qualif | qualif;
@@ -221,46 +243,85 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
             return;
         }
 
-        let mut dest = dest;
-        let index = loop {
-            match dest {
-                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;
+        if self.const_let_allowed() {
+            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),
             }
-        };
-        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);
+                store(&mut self.local_qualif[index])
+            }
+
+            Place::Projection(box Projection {
+                base: Place::Local(index),
+                elem: ProjectionElem::Deref
+            }) if self.mir.local_kind(index) == LocalKind::Temp
+               && self.mir.local_decls[index].ty.is_box()
+               && self.local_qualif[index].map_or(false, |qualif| {
+                    qualif.contains(Qualif::NOT_CONST)
+               }) => {
+                // Part of `box expr`, we should've errored
+                // already for the Box allocation Rvalue.
+            }
+
+            // This must be an explicit assignment.
+            _ => {
+                // Catch more errors in the destination.
+                self.visit_place(
+                    dest,
+                    PlaceContext::MutatingUse(MutatingUseContext::Store),
+                    location
+                );
+                self.statement_like();
+            }
+        }
+    }
+
+    fn const_let_allowed(&self) -> bool {
+        self.tcx.features().const_let
     }
 
     /// Qualify a whole const, static initializer or const fn.
@@ -299,7 +360,48 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
                 TerminatorKind::FalseEdges { .. } |
                 TerminatorKind::FalseUnwind { .. } => None,
 
-                TerminatorKind::Return => break,
+                TerminatorKind::Return => {
+                    if !self.const_let_allowed() {
+                        // Check for unused values. This usually means
+                        // there are extra statements in the AST.
+                        for temp in mir.temps_iter() {
+                            if self.local_qualif[temp].is_none() {
+                                continue;
+                            }
+
+                            let state = self.temp_promotion_state[temp];
+                            if let TempState::Defined { location, uses: 0 } = state {
+                                let data = &mir[location.block];
+                                let stmt_idx = location.statement_index;
+
+                                // Get the span for the initialization.
+                                let source_info = if stmt_idx < data.statements.len() {
+                                    data.statements[stmt_idx].source_info
+                                } else {
+                                    data.terminator().source_info
+                                };
+                                self.span = source_info.span;
+
+                                // Treat this as a statement in the AST.
+                                self.statement_like();
+                            }
+                        }
+
+                        // Make sure there are no extra unassigned variables.
+                        self.qualif = Qualif::NOT_CONST;
+                        for index in mir.vars_iter() {
+                            if !self.const_fn_arg_vars.contains(index) {
+                                debug!("unassigned variable {:?}", index);
+                                self.assign(&Place::Local(index), Location {
+                                    block: bb,
+                                    statement_index: usize::MAX,
+                                });
+                            }
+                        }
+                    }
+
+                    break;
+                }
             };
 
             match target {
@@ -366,6 +468,14 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
             LocalKind::ReturnPointer => {
                 self.not_const();
             }
+            LocalKind::Var if !self.const_let_allowed() => {
+                if self.mode != Mode::Fn {
+                    emit_feature_err(&self.tcx.sess.parse_sess, "const_let",
+                                    self.span, GateIssue::Language,
+                                    &format!("let bindings in {}s are unstable",self.mode));
+                }
+                self.add(Qualif::NOT_CONST);
+            }
             LocalKind::Var |
             LocalKind::Arg |
             LocalKind::Temp => {
@@ -448,6 +558,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                         ProjectionElem::Deref => {
                             if context.is_mutating_use() {
                                 this.not_const()
+                            } else {
+                                this.qualif = Qualif::NOT_CONST;
                             }
                             let base_ty = proj.base.ty(this.mir, this.tcx).to_ty(this.tcx);
                             match this.mode {
@@ -1050,6 +1162,46 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
         debug!("visit_assign: dest={:?} rvalue={:?} location={:?}", dest, rvalue, location);
         self.visit_rvalue(rvalue, location);
 
+        // Check the allowed const fn argument forms.
+        if let (Mode::ConstFn, &Place::Local(index)) = (self.mode, dest) {
+            if self.mir.local_kind(index) == LocalKind::Var &&
+               self.const_fn_arg_vars.insert(index) &&
+               !self.tcx.sess.features_untracked().const_let {
+                // Direct use of an argument is permitted.
+                match *rvalue {
+                    Rvalue::Use(Operand::Copy(Place::Local(local))) |
+                    Rvalue::Use(Operand::Move(Place::Local(local))) => {
+                        if self.mir.local_kind(local) == LocalKind::Arg {
+                            return;
+                        }
+                    }
+                    _ => {}
+                }
+                // Avoid a generic error for other uses of arguments.
+                if self.qualif.contains(Qualif::FN_ARGUMENT) {
+                    let decl = &self.mir.local_decls[index];
+                    let mut err = feature_err(
+                        &self.tcx.sess.parse_sess,
+                        "const_let",
+                        decl.source_info.span,
+                        GateIssue::Language,
+                        "arguments of constant functions can only be immutable by-value bindings"
+                    );
+                    if self.tcx.sess.teach(&err.get_code().unwrap()) {
+                        err.note("Constant functions are not allowed to mutate anything. Thus, \
+                                  binding to an argument with a mutable pattern is not allowed.");
+                        err.note("Remove any mutable bindings from the argument list to fix this \
+                                  error. In case you need to mutate the argument, try lazily \
+                                  initializing a global variable instead of using a const fn, or \
+                                  refactoring the code to a functional style to avoid mutation if \
+                                  possible.");
+                    }
+                    err.emit();
+                    return;
+                }
+            }
+        }
+
         self.assign(dest, location);
     }
 

src/librustc_mir/transform/qualify_min_const_fn.rs

@@ -64,6 +64,12 @@ pub fn is_min_const_fn(
         }
     }
 
+    for local in mir.vars_iter() {
+        return Err((
+            mir.local_decls[local].source_info.span,
+            "local variables in const fn are unstable".into(),
+        ));
+    }
     for local in &mir.local_decls {
         check_ty(tcx, local.ty, local.source_info.span)?;
     }
@@ -264,8 +270,15 @@ fn check_place(
     mode: PlaceMode,
 ) -> McfResult {
     match place {
-        // assignments to locals, arguments, temporaries or the return slot are fine
-        Place::Local(_) => Ok(()),
+        Place::Local(l) => match mode {
+            PlaceMode::Assign => match mir.local_kind(*l) {
+                LocalKind::Temp | LocalKind::ReturnPointer => Ok(()),
+                LocalKind::Arg | LocalKind::Var => {
+                    Err((span, "assignments in const fn are unstable".into()))
+                }
+            },
+            PlaceMode::Read => Ok(()),
+        },
         // promoteds are always fine, they are essentially constants
         Place::Promoted(_) => Ok(()),
         Place::Static(_) => Err((span, "cannot access `static` items in const fn".into())),

src/libsyntax/feature_gate.rs

@@ -209,6 +209,9 @@ declare_features! (
     // Allows the definition of `const fn` functions with some advanced features.
     (active, const_fn, "1.2.0", Some(24111), None),
 
+    // Allows let bindings and destructuring in `const fn` functions and constants.
+    (active, const_let, "1.22.1", Some(48821), None),
+
     // Allows accessing fields of unions inside const fn.
     (active, const_fn_union, "1.27.0", Some(51909), None),
 

src/test/run-pass/ctfe/const-block-non-item-statement-3.rs

@@ -11,6 +11,8 @@
 // run-pass
 #![allow(dead_code)]
 
+#![feature(const_let)]
+
 type Array = [u32; {  let x = 2; 5 }];
 
 pub fn main() {}

src/test/run-pass/ctfe/const-block-non-item-statement.rs

@@ -11,6 +11,8 @@
 // run-pass
 #![allow(dead_code)]
 
+#![feature(const_let)]
+
 enum Foo {
     Bar = { let x = 1; 3 }
 }

src/test/run-pass/ctfe/locals-in-const-fn.rs

@@ -12,7 +12,7 @@
 
 // https://github.com/rust-lang/rust/issues/48821
 
-#![feature(const_fn)]
+#![feature(const_fn, const_let)]
 
 const fn foo(i: usize) -> usize {
     let x = i;