Auto merge of #63118 - Centril:stabilize-bind-by-move, r=matthewjasper

Stabilize `bind_by_move_pattern_guards` in Rust 1.39.0

Closes #15287.

After stabilizing `#![feature(bind_by_move_pattern_guards)]`, you can now use bind-by-move bindings in patterns and take references to those bindings in `if` guards of `match` expressions. For example, the following now becomes legal:

```rust
fn main() {
    let array: Box<[u8; 4]> = Box::new([1, 2, 3, 4]);

    match array {
        nums
//      ---- `nums` is bound by move.
            if nums.iter().sum::<u8>() == 10
//                 ^------ `.iter()` implicitly takes a reference to `nums`.
        => {
            drop(nums);
//          --------- Legal as `nums` was bound by move and so we have ownership.
        }
        _ => unreachable!(),
    }
}
```

r? @matthewjasper

Author: bors

src/libcore/lib.rs

@@ -87,7 +87,7 @@
 #![feature(link_llvm_intrinsics)]
 #![feature(never_type)]
 #![feature(nll)]
-#![feature(bind_by_move_pattern_guards)]
+#![cfg_attr(boostrap_stdarch_ignore_this, feature(bind_by_move_pattern_guards))]
 #![feature(exhaustive_patterns)]
 #![feature(no_core)]
 #![feature(on_unimplemented)]

src/librustc_mir/build/matches/mod.rs

@@ -1345,13 +1345,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// any, and then branches to the arm. Returns the block for the case where
     /// the guard fails.
     ///
-    /// Note: we check earlier that if there is a guard, there cannot be move
-    /// bindings (unless feature(bind_by_move_pattern_guards) is used). This
-    /// isn't really important for the self-consistency of this fn, but the
-    /// reason for it should be clear: after we've done the assignments, if
-    /// there were move bindings, further tests would be a use-after-move.
-    /// bind_by_move_pattern_guards avoids this by only moving the binding once
-    /// the guard has evaluated to true (see below).
+    /// Note: we do not check earlier that if there is a guard,
+    /// there cannot be move bindings. We avoid a use-after-move by only
+    /// moving the binding once the guard has evaluated to true (see below).
     fn bind_and_guard_matched_candidate<'pat>(
         &mut self,
         candidate: Candidate<'pat, 'tcx>,

src/librustc_mir/error_codes.rs

@@ -157,81 +157,6 @@ match x {
 See also the error E0303.
 "##,
 
-E0008: r##"
-Names bound in match arms retain their type in pattern guards. As such, if a
-name is bound by move in a pattern, it should also be moved to wherever it is
-referenced in the pattern guard code. Doing so however would prevent the name
-from being available in the body of the match arm. Consider the following:
-
-```compile_fail,E0008
-match Some("hi".to_string()) {
-    Some(s) if s.len() == 0 => {}, // use s.
-    _ => {},
-}
-```
-
-The variable `s` has type `String`, and its use in the guard is as a variable of
-type `String`. The guard code effectively executes in a separate scope to the
-body of the arm, so the value would be moved into this anonymous scope and
-therefore becomes unavailable in the body of the arm.
-
-The problem above can be solved by using the `ref` keyword.
-
-```
-match Some("hi".to_string()) {
-    Some(ref s) if s.len() == 0 => {},
-    _ => {},
-}
-```
-
-Though this example seems innocuous and easy to solve, the problem becomes clear
-when it encounters functions which consume the value:
-
-```compile_fail,E0008
-struct A{}
-
-impl A {
-    fn consume(self) -> usize {
-        0
-    }
-}
-
-fn main() {
-    let a = Some(A{});
-    match a {
-        Some(y) if y.consume() > 0 => {}
-        _ => {}
-    }
-}
-```
-
-In this situation, even the `ref` keyword cannot solve it, since borrowed
-content cannot be moved. This problem cannot be solved generally. If the value
-can be cloned, here is a not-so-specific solution:
-
-```
-#[derive(Clone)]
-struct A{}
-
-impl A {
-    fn consume(self) -> usize {
-        0
-    }
-}
-
-fn main() {
-    let a = Some(A{});
-    match a{
-        Some(ref y) if y.clone().consume() > 0 => {}
-        _ => {}
-    }
-}
-```
-
-If the value will be consumed in the pattern guard, using its clone will not
-move its ownership, so the code works.
-"##,
-
 E0009: r##"
 In a pattern, all values that don't implement the `Copy` trait have to be bound
 the same way. The goal here is to avoid binding simultaneously by-move and
@@ -475,13 +400,15 @@ for item in xs {
 "##,
 
 E0301: r##"
+#### Note: this error code is no longer emitted by the compiler.
+
 Mutable borrows are not allowed in pattern guards, because matching cannot have
 side effects. Side effects could alter the matched object or the environment
 on which the match depends in such a way, that the match would not be
 exhaustive. For instance, the following would not match any arm if mutable
 borrows were allowed:
 
-```compile_fail,E0301
+```compile_fail,E0596
 match Some(()) {
     None => { },
     option if option.take().is_none() => {
@@ -493,13 +420,15 @@ match Some(()) {
 "##,
 
 E0302: r##"
+#### Note: this error code is no longer emitted by the compiler.
+
 Assignments are not allowed in pattern guards, because matching cannot have
 side effects. Side effects could alter the matched object or the environment
 on which the match depends in such a way, that the match would not be
 exhaustive. For instance, the following would not match any arm if assignments
 were allowed:
 
-```compile_fail,E0302
+```compile_fail,E0594
 match Some(()) {
     None => { },
     option if { option = None; false } => { },
@@ -1989,7 +1918,6 @@ When matching on a variable it cannot be mutated in the match guards, as this
 could cause the match to be non-exhaustive:
 
 ```compile_fail,E0510
-#![feature(bind_by_move_pattern_guards)]
 let mut x = Some(0);
 match x {
     None => (),
@@ -2451,6 +2379,7 @@ There are some known bugs that trigger this message.
 
 ;
 
+//  E0008, // cannot bind by-move into a pattern guard
 //  E0298, // cannot compare constants
 //  E0299, // mismatched types between arms
 //  E0471, // constant evaluation error (in pattern)

src/librustc_mir/hair/pattern/check_match.rs

@@ -5,11 +5,6 @@ use super::_match::WitnessPreference::*;
 use super::{Pattern, PatternContext, PatternError, PatternKind};
 
 use rustc::middle::borrowck::SignalledError;
-use rustc::middle::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor};
-use rustc::middle::expr_use_visitor::{LoanCause, MutateMode};
-use rustc::middle::expr_use_visitor as euv;
-use rustc::middle::mem_categorization::cmt_;
-use rustc::middle::region;
 use rustc::session::Session;
 use rustc::ty::{self, Ty, TyCtxt};
 use rustc::ty::subst::{InternalSubsts, SubstsRef};
@@ -36,9 +31,7 @@ crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) -> SignalledError {
 
     let mut visitor = MatchVisitor {
         tcx,
-        body_owner: def_id,
         tables: tcx.body_tables(body_id),
-        region_scope_tree: &tcx.region_scope_tree(def_id),
         param_env: tcx.param_env(def_id),
         identity_substs: InternalSubsts::identity_for_item(tcx, def_id),
         signalled_error: SignalledError::NoErrorsSeen,
@@ -53,11 +46,9 @@ fn create_e0004(sess: &Session, sp: Span, error_message: String) -> DiagnosticBu
 
 struct MatchVisitor<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
-    body_owner: DefId,
     tables: &'a ty::TypeckTables<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     identity_substs: SubstsRef<'tcx>,
-    region_scope_tree: &'a region::ScopeTree,
     signalled_error: SignalledError,
 }
 
@@ -151,11 +142,8 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
 
             // Second, if there is a guard on each arm, make sure it isn't
             // assigning or borrowing anything mutably.
-            if let Some(ref guard) = arm.guard {
+            if arm.guard.is_some() {
                 self.signalled_error = SignalledError::SawSomeError;
-                if !self.tcx.features().bind_by_move_pattern_guards {
-                    check_for_mutation_in_guard(self, &guard);
-                }
             }
 
             // Third, perform some lints.
@@ -582,19 +570,10 @@ fn check_legality_of_move_bindings(
                              "cannot bind by-move with sub-bindings")
                 .span_label(p.span, "binds an already bound by-move value by moving it")
                 .emit();
-        } else if has_guard {
-            if !cx.tcx.features().bind_by_move_pattern_guards {
-                let mut err = struct_span_err!(cx.tcx.sess, p.span, E0008,
-                                            "cannot bind by-move into a pattern guard");
-                err.span_label(p.span, "moves value into pattern guard");
-                if cx.tcx.sess.opts.unstable_features.is_nightly_build() {
-                    err.help("add `#![feature(bind_by_move_pattern_guards)]` to the \
-                            crate attributes to enable");
-                }
-                err.emit();
+        } else if !has_guard {
+            if let Some(_by_ref_span) = by_ref_span {
+                span_vec.push(p.span);
             }
-        } else if let Some(_by_ref_span) = by_ref_span {
-            span_vec.push(p.span);
         }
     };
 
@@ -636,67 +615,6 @@ fn check_legality_of_move_bindings(
     }
 }
 
-/// Ensures that a pattern guard doesn't borrow by mutable reference or assign.
-//
-// FIXME: this should be done by borrowck.
-fn check_for_mutation_in_guard(cx: &MatchVisitor<'_, '_>, guard: &hir::Guard) {
-    let mut checker = MutationChecker {
-        cx,
-    };
-    match guard {
-        hir::Guard::If(expr) =>
-            ExprUseVisitor::new(&mut checker,
-                                cx.tcx,
-                                cx.body_owner,
-                                cx.param_env,
-                                cx.region_scope_tree,
-                                cx.tables,
-                                None).walk_expr(expr),
-    };
-}
-
-struct MutationChecker<'a, 'tcx> {
-    cx: &'a MatchVisitor<'a, 'tcx>,
-}
-
-impl<'a, 'tcx> Delegate<'tcx> for MutationChecker<'a, 'tcx> {
-    fn matched_pat(&mut self, _: &Pat, _: &cmt_<'_>, _: euv::MatchMode) {}
-    fn consume(&mut self, _: hir::HirId, _: Span, _: &cmt_<'_>, _: ConsumeMode) {}
-    fn consume_pat(&mut self, _: &Pat, _: &cmt_<'_>, _: ConsumeMode) {}
-    fn borrow(&mut self,
-              _: hir::HirId,
-              span: Span,
-              _: &cmt_<'_>,
-              _: ty::Region<'tcx>,
-              kind:ty:: BorrowKind,
-              _: LoanCause) {
-        match kind {
-            ty::MutBorrow => {
-                let mut err = struct_span_err!(self.cx.tcx.sess, span, E0301,
-                          "cannot mutably borrow in a pattern guard");
-                err.span_label(span, "borrowed mutably in pattern guard");
-                if self.cx.tcx.sess.opts.unstable_features.is_nightly_build() {
-                    err.help("add `#![feature(bind_by_move_pattern_guards)]` to the \
-                              crate attributes to enable");
-                }
-                err.emit();
-            }
-            ty::ImmBorrow | ty::UniqueImmBorrow => {}
-        }
-    }
-    fn decl_without_init(&mut self, _: hir::HirId, _: Span) {}
-    fn mutate(&mut self, _: hir::HirId, span: Span, _: &cmt_<'_>, mode: MutateMode) {
-        match mode {
-            MutateMode::JustWrite | MutateMode::WriteAndRead => {
-                struct_span_err!(self.cx.tcx.sess, span, E0302, "cannot assign in a pattern guard")
-                    .span_label(span, "assignment in pattern guard")
-                    .emit();
-            }
-            MutateMode::Init => {}
-        }
-    }
-}
-
 /// Forbids bindings in `@` patterns. This is necessary for memory safety,
 /// because of the way rvalues are handled in the borrow check. (See issue
 /// #14587.)

src/librustc_passes/lib.rs

@@ -8,7 +8,7 @@
 
 #![feature(in_band_lifetimes)]
 #![feature(nll)]
-#![feature(bind_by_move_pattern_guards)]
+#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]
 
 #![recursion_limit="256"]
 

src/librustdoc/lib.rs

@@ -1,7 +1,7 @@
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/",
        html_playground_url = "https://play.rust-lang.org/")]
 
-#![feature(bind_by_move_pattern_guards)]
+#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]
 #![feature(rustc_private)]
 #![feature(arbitrary_self_types)]
 #![feature(box_patterns)]

src/libstd/lib.rs

@@ -238,7 +238,7 @@
 #![feature(array_error_internals)]
 #![feature(asm)]
 #![feature(associated_type_bounds)]
-#![feature(bind_by_move_pattern_guards)]
+#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]
 #![feature(box_syntax)]
 #![feature(c_variadic)]
 #![feature(cfg_target_has_atomic)]

src/libsyntax/feature_gate/accepted.rs

@@ -241,6 +241,8 @@ declare_features! (
     (accepted, underscore_const_names, "1.37.0", Some(54912), None),
     /// Allows free and inherent `async fn`s, `async` blocks, and `<expr>.await` expressions.
     (accepted, async_await, "1.39.0", Some(50547), None),
+    /// Allows mixing bind-by-move in patterns and references to those identifiers in guards.
+    (accepted, bind_by_move_pattern_guards, "1.39.0", Some(15287), None),
 
     // -------------------------------------------------------------------------
     // feature-group-end: accepted features

src/libsyntax/feature_gate/active.rs

@@ -461,9 +461,6 @@ declare_features! (
     /// Allows non-builtin attributes in inner attribute position.
     (active, custom_inner_attributes, "1.30.0", Some(54726), None),
 
-    /// Allows mixing bind-by-move in patterns and references to those identifiers in guards.
-    (active, bind_by_move_pattern_guards, "1.30.0", Some(15287), None),
-
     /// Allows `impl Trait` in bindings (`let`, `const`, `static`).
     (active, impl_trait_in_bindings, "1.30.0", Some(63065), None),
 

src/libsyntax/lib.rs

@@ -7,7 +7,7 @@
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/",
        test(attr(deny(warnings))))]
 
-#![feature(bind_by_move_pattern_guards)]
+#![cfg_attr(bootstrap, feature(bind_by_move_pattern_guards))]
 #![feature(box_syntax)]
 #![feature(const_fn)]
 #![feature(const_transmute)]