Auto merge of #57609 - matthewjasper:more-restrictive-match, r=pnkfelix

Use normal mutable borrows in matches

`ref mut` borrows are currently two-phase with NLL enabled. This changes them to be proper mutable borrows. To accommodate this, first the position of fake borrows is changed:

```text
[ 1. Pre-match ]
       |
[ (old create fake borrows) ]
[ 2. Discriminant testing -- check discriminants ] <-+
       |                                             |
       | (once a specific arm is chosen)             |
       |                                             |
[ (old read fake borrows) ]                          |
[ 3. Create "guard bindings" for arm ]               |
[ (create fake borrows) ]                            |
       |                                             |
[ 4. Execute guard code ]                            |
[ (read fake borrows) ] --(guard is false)-----------+
       |
       | (guard results in true)
       |
[ 5. Create real bindings and execute arm ]
       |
[ Exit match ]
```

The following additional changes are made to accommodate `ref mut` bindings:

* We no longer create fake `Shared` borrows. These borrows are no longer needed for soundness, just to avoid some arguably strange cases.
* `Shallow` borrows no longer conflict with existing borrows, avoiding conflicting access between the guard borrow access and the `ref mut` borrow.

There is some further clean up done in this PR:

* Avoid the "later used here" note for Shallow borrows (since it's not relevant with the message provided)
* Make any use of a two-phase borrow activate it.
* Simplify the cleanup_post_borrowck passes into a single pass.

cc #56254

r? @nikomatsakis

Author: bors

src/librustc/ich/impls_mir.rs

@@ -196,7 +196,12 @@ impl_stable_hash_for!(impl<'gcx> for enum mir::StatementKind<'gcx> [ mir::Statem
 });
 
 impl_stable_hash_for!(enum mir::RetagKind { FnEntry, TwoPhase, Raw, Default });
-impl_stable_hash_for!(enum mir::FakeReadCause { ForMatchGuard, ForMatchedPlace, ForLet });
+impl_stable_hash_for!(enum mir::FakeReadCause {
+    ForMatchGuard,
+    ForMatchedPlace,
+    ForGuardBinding,
+    ForLet
+});
 
 impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for mir::Place<'gcx> {
     fn hash_stable<W: StableHasherResult>(&self,

src/librustc/mir/mod.rs

@@ -1823,17 +1823,22 @@ pub enum RetagKind {
 /// The `FakeReadCause` describes the type of pattern why a `FakeRead` statement exists.
 #[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)]
 pub enum FakeReadCause {
-    /// Inject a fake read of the borrowed input at the start of each arm's
-    /// pattern testing code.
+    /// Inject a fake read of the borrowed input at the end of each guards
+    /// code.
     ///
-    /// This should ensure that you cannot change the variant for an enum
-    /// while you are in the midst of matching on it.
+    /// This should ensure that you cannot change the variant for an enum while
+    /// you are in the midst of matching on it.
     ForMatchGuard,
 
     /// `let x: !; match x {}` doesn't generate any read of x so we need to
     /// generate a read of x to check that it is initialized and safe.
     ForMatchedPlace,
 
+    /// A fake read of the RefWithinGuard version of a bind-by-value variable
+    /// in a match guard to ensure that it's value hasn't change by the time
+    /// we create the OutsideGuard version.
+    ForGuardBinding,
+
     /// Officially, the semantics of
     ///
     /// `let pattern = <expr>;`

src/librustc_mir/borrow_check/borrow_set.rs

@@ -3,9 +3,7 @@ use crate::borrow_check::nll::ToRegionVid;
 use crate::dataflow::indexes::BorrowIndex;
 use crate::dataflow::move_paths::MoveData;
 use rustc::mir::traversal;
-use rustc::mir::visit::{
-    PlaceContext, Visitor, NonUseContext, MutatingUseContext, NonMutatingUseContext
-};
+use rustc::mir::visit::{PlaceContext, Visitor, NonUseContext, MutatingUseContext};
 use rustc::mir::{self, Location, Mir, Local};
 use rustc::ty::{RegionVid, TyCtxt};
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
@@ -257,31 +255,21 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
                 );
             }
 
-            // Otherwise, this is the unique later use
-            // that we expect.
-            borrow_data.activation_location = match context {
-                // The use of TMP in a shared borrow does not
-                // count as an actual activation.
-                PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) |
-                PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) =>
-                    TwoPhaseActivation::NotActivated,
-                _ => {
-                    // Double check: This borrow is indeed a two-phase borrow (that is,
-                    // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
-                    // we've not found any other activations (checked above).
-                    assert_eq!(
-                        borrow_data.activation_location,
-                        TwoPhaseActivation::NotActivated,
-                        "never found an activation for this borrow!",
-                    );
-
-                    self.activation_map
-                        .entry(location)
-                        .or_default()
-                        .push(borrow_index);
-                    TwoPhaseActivation::ActivatedAt(location)
-                }
-            };
+            // Otherwise, this is the unique later use that we expect.
+            // Double check: This borrow is indeed a two-phase borrow (that is,
+            // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
+            // we've not found any other activations (checked above).
+            assert_eq!(
+                borrow_data.activation_location,
+                TwoPhaseActivation::NotActivated,
+                "never found an activation for this borrow!",
+            );
+            self.activation_map
+                .entry(location)
+                .or_default()
+                .push(borrow_index);
+
+            borrow_data.activation_location = TwoPhaseActivation::ActivatedAt(location);
         }
     }
 

src/librustc_mir/borrow_check/error_reporting.rs

@@ -1336,22 +1336,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         let loan_span = loan_spans.args_or_use();
 
         let tcx = self.infcx.tcx;
-        let mut err = if loan.kind == BorrowKind::Shallow {
-            tcx.cannot_mutate_in_match_guard(
+        if loan.kind == BorrowKind::Shallow {
+            let mut err = tcx.cannot_mutate_in_match_guard(
                 span,
                 loan_span,
                 &self.describe_place(place).unwrap_or_else(|| "_".to_owned()),
                 "assign",
                 Origin::Mir,
-            )
-        } else {
-            tcx.cannot_assign_to_borrowed(
-                span,
-                loan_span,
-                &self.describe_place(place).unwrap_or_else(|| "_".to_owned()),
-                Origin::Mir,
-            )
-        };
+            );
+            loan_spans.var_span_label(
+                &mut err,
+                format!("borrow occurs due to use{}", loan_spans.describe()),
+            );
+
+            err.buffer(&mut self.errors_buffer);
+
+            return;
+        }
+
+        let mut err = tcx.cannot_assign_to_borrowed(
+            span,
+            loan_span,
+            &self.describe_place(place).unwrap_or_else(|| "_".to_owned()),
+            Origin::Mir,
+        );
 
         loan_spans.var_span_label(
             &mut err,

src/librustc_mir/borrow_check/mod.rs

@@ -998,7 +998,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 }
 
                 (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
-                | (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) => {
+                | (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
+                | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique)
+                | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
                     Control::Continue
                 }
 

src/librustc_mir/borrow_check/nll/invalidation.rs

@@ -78,13 +78,8 @@ impl<'cx, 'tcx, 'gcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx, 'gcx> {
                     JustWrite
                 );
             }
-            StatementKind::FakeRead(_, ref place) => {
-                self.access_place(
-                    ContextKind::FakeRead.new(location),
-                    place,
-                    (Deep, Read(ReadKind::Borrow(BorrowKind::Shared))),
-                    LocalMutationIsAllowed::No,
-                );
+            StatementKind::FakeRead(_, _) => {
+                // Only relavent for initialized/liveness/safety checks.
             }
             StatementKind::SetDiscriminant {
                 ref place,
@@ -438,7 +433,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
                     }
 
                     (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
-                    | (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
+                    | (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
+                    | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique)
+                    | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
                         // Reads/reservations don't invalidate shared or shallow borrows
                     }
 

src/librustc_mir/build/block.rs

@@ -6,8 +6,6 @@ use rustc::mir::*;
 use rustc::hir;
 use syntax_pos::Span;
 
-use std::slice;
-
 impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     pub fn ast_block(&mut self,
                      destination: &Place<'tcx>,
@@ -125,7 +123,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                             None,
                             remainder_span,
                             lint_level,
-                            slice::from_ref(&pattern),
+                            &pattern,
                             ArmHasGuard(false),
                             Some((None, initializer_span)),
                         );
@@ -138,7 +136,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                             }));
                     } else {
                         scope = this.declare_bindings(
-                            None, remainder_span, lint_level, slice::from_ref(&pattern),
+                            None, remainder_span, lint_level, &pattern,
                             ArmHasGuard(false), None);
 
                         debug!("ast_block_stmts: pattern={:?}", pattern);

src/librustc_mir/build/expr/into.rs

@@ -53,8 +53,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             ExprKind::Block { body: ast_block } => {
                 this.ast_block(destination, block, ast_block, source_info)
             }
-            ExprKind::Match { discriminant, arms } => {
-                this.match_expr(destination, expr_span, block, discriminant, arms)
+            ExprKind::Match { scrutinee, arms } => {
+                this.match_expr(destination, expr_span, block, scrutinee, arms)
             }
             ExprKind::NeverToAny { source } => {
                 let source = this.hir.mirror(source);