Unrolled build for rust-lang#123324

Rollup merge of rust-lang#123324 - Nadrieril:false-edges2, r=matthewjasper

match lowering: make false edges more precise

When lowering match expressions, we add false edges to hide details of the lowering from borrowck. Morally we pretend we're testing the patterns (and guards) one after the other in order. See the tests for examples. Problem is, the way we implement this today is too coarse for deref patterns.

In deref patterns, a pattern like `deref [1, x]` matches on a `Vec` by creating a temporary to store the output of the call to `deref()` and then uses that to continue matching. Here the pattern has a binding, which we set up after the pre-binding block. Problem is, currently the false edges tell borrowck that the pre-binding block can be reached from a previous arm as well, so the `deref()` temporary may not be initialized. This triggers an error when we try to use the binding `x`.

We could call `deref()` a second time, but this opens the door to soundness issues if the deref impl is weird. Instead in this PR I rework false edges a little bit.

What we need from false edges is a (fake) path from each candidate to the next, specifically from candidate C's pre-binding block to next candidate D's pre-binding block. Today, we link the pre-binding blocks directly. In this PR, I link them indirectly by choosing an earlier node on D's success path. Specifically, I choose the earliest block on D's success path that doesn't make a loop (if I chose e.g. the start block of the whole match (which is on the success path of all candidates), that would make a loop). This turns out to be rather straightforward to implement.

r? `@matthewjasper` if you have the bandwidth, otherwise let me know

Author: rust-timer

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -214,12 +214,77 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///
     /// ## False edges
     ///
-    /// We don't want to have the exact structure of the decision tree be
-    /// visible through borrow checking. False edges ensure that the CFG as
-    /// seen by borrow checking doesn't encode this. False edges are added:
+    /// We don't want to have the exact structure of the decision tree be visible through borrow
+    /// checking. Specifically we want borrowck to think that:
+    /// - at any point, any or none of the patterns and guards seen so far may have been tested;
+    /// - after the match, any of the patterns may have matched.
     ///
-    /// * From each pre-binding block to the next pre-binding block.
-    /// * From each otherwise block to the next pre-binding block.
+    /// For example, all of these would fail to error if borrowck could see the real CFG (examples
+    /// taken from `tests/ui/nll/match-cfg-fake-edges.rs`):
+    /// ```ignore (too many errors, this is already in the test suite)
+    /// let x = String::new();
+    /// let _ = match true {
+    ///     _ => {},
+    ///     _ => drop(x),
+    /// };
+    /// // Borrowck must not know the second arm is never run.
+    /// drop(x); //~ ERROR use of moved value
+    ///
+    /// let x;
+    /// # let y = true;
+    /// match y {
+    ///     _ if { x = 2; true } => {},
+    ///     // Borrowck must not know the guard is always run.
+    ///     _ => drop(x), //~ ERROR used binding `x` is possibly-uninitialized
+    /// };
+    ///
+    /// let x = String::new();
+    /// # let y = true;
+    /// match y {
+    ///     false if { drop(x); true } => {},
+    ///     // Borrowck must not know the guard is not run in the `true` case.
+    ///     true => drop(x), //~ ERROR use of moved value: `x`
+    ///     false => {},
+    /// };
+    ///
+    /// # let mut y = (true, true);
+    /// let r = &mut y.1;
+    /// match y {
+    ///     //~^ ERROR cannot use `y.1` because it was mutably borrowed
+    ///     (false, true) => {}
+    ///     // Borrowck must not know we don't test `y.1` when `y.0` is `true`.
+    ///     (true, _) => drop(r),
+    ///     (false, _) => {}
+    /// };
+    /// ```
+    ///
+    /// We add false edges to act as if we were naively matching each arm in order. What we need is
+    /// a (fake) path from each candidate to the next, specifically from candidate C's pre-binding
+    /// block to next candidate D's pre-binding block. For maximum precision (needed for deref
+    /// patterns), we choose the earliest node on D's success path that doesn't also lead to C (to
+    /// avoid loops).
+    ///
+    /// This turns out to be easy to compute: that block is the `start_block` of the first call to
+    /// `match_candidates` where D is the first candidate in the list.
+    ///
+    /// For example:
+    /// ```rust
+    /// # let (x, y) = (true, true);
+    /// match (x, y) {
+    ///   (true, true) => 1,
+    ///   (false, true) => 2,
+    ///   (true, false) => 3,
+    ///   _ => 4,
+    /// }
+    /// # ;
+    /// ```
+    /// In this example, the pre-binding block of arm 1 has a false edge to the block for result
+    /// `false` of the first test on `x`. The other arms have false edges to the pre-binding blocks
+    /// of the next arm.
+    ///
+    /// On top of this, we also add a false edge from the otherwise_block of each guard to the
+    /// aforementioned start block of the next candidate, to ensure borrock doesn't rely on which
+    /// guards may have run.
     #[instrument(level = "debug", skip(self, arms))]
     pub(crate) fn match_expr(
         &mut self,
@@ -365,7 +430,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         for candidate in candidates {
             candidate.visit_leaves(|leaf_candidate| {
                 if let Some(ref mut prev) = previous_candidate {
-                    prev.next_candidate_pre_binding_block = leaf_candidate.pre_binding_block;
+                    assert!(leaf_candidate.false_edge_start_block.is_some());
+                    prev.next_candidate_start_block = leaf_candidate.false_edge_start_block;
                 }
                 previous_candidate = Some(leaf_candidate);
             });
@@ -1010,8 +1076,12 @@ struct Candidate<'pat, 'tcx> {
 
     /// The block before the `bindings` have been established.
     pre_binding_block: Option<BasicBlock>,
-    /// The pre-binding block of the next candidate.
-    next_candidate_pre_binding_block: Option<BasicBlock>,
+
+    /// The earliest block that has only candidates >= this one as descendents. Used for false
+    /// edges, see the doc for [`Builder::match_expr`].
+    false_edge_start_block: Option<BasicBlock>,
+    /// The `false_edge_start_block` of the next candidate.
+    next_candidate_start_block: Option<BasicBlock>,
 }
 
 impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
@@ -1033,7 +1103,8 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
             or_span: None,
             otherwise_block: None,
             pre_binding_block: None,
-            next_candidate_pre_binding_block: None,
+            false_edge_start_block: None,
+            next_candidate_start_block: None,
         }
     }
 
@@ -1325,6 +1396,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         otherwise_block: BasicBlock,
         candidates: &mut [&mut Candidate<'_, 'tcx>],
     ) {
+        if let [first, ..] = candidates {
+            if first.false_edge_start_block.is_none() {
+                first.false_edge_start_block = Some(start_block);
+            }
+        }
+
         match candidates {
             [] => {
                 // If there are no candidates that still need testing, we're done. Since all matches are
@@ -1545,6 +1622,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             .into_iter()
             .map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard))
             .collect();
+        candidate.subcandidates[0].false_edge_start_block = candidate.false_edge_start_block;
     }
 
     /// Try to merge all of the subcandidates of the given candidate into one. This avoids
@@ -1564,6 +1642,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             let any_matches = self.cfg.start_new_block();
             let or_span = candidate.or_span.take().unwrap();
             let source_info = self.source_info(or_span);
+            if candidate.false_edge_start_block.is_none() {
+                candidate.false_edge_start_block =
+                    candidate.subcandidates[0].false_edge_start_block;
+            }
             for subcandidate in mem::take(&mut candidate.subcandidates) {
                 let or_block = subcandidate.pre_binding_block.unwrap();
                 self.cfg.goto(or_block, source_info, any_matches);
@@ -1979,12 +2061,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         let mut block = candidate.pre_binding_block.unwrap();
 
-        if candidate.next_candidate_pre_binding_block.is_some() {
+        if candidate.next_candidate_start_block.is_some() {
             let fresh_block = self.cfg.start_new_block();
             self.false_edges(
                 block,
                 fresh_block,
-                candidate.next_candidate_pre_binding_block,
+                candidate.next_candidate_start_block,
                 candidate_source_info,
             );
             block = fresh_block;
@@ -2132,7 +2214,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             self.false_edges(
                 otherwise_post_guard_block,
                 otherwise_block,
-                candidate.next_candidate_pre_binding_block,
+                candidate.next_candidate_start_block,
                 source_info,
             );
 

tests/mir-opt/building/match/match_false_edges.main.built.after.mir

@@ -48,7 +48,7 @@ fn main() -> () {
     }
 
     bb2: {
-        falseEdge -> [real: bb15, imaginary: bb6];
+        falseEdge -> [real: bb15, imaginary: bb3];
     }
 
     bb3: {

tests/mir-opt/building/match/sort_candidates.constant_eq.SimplifyCfg-initial.after.mir

@@ -40,15 +40,15 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
     }
 
     bb4: {
-        falseEdge -> [real: bb12, imaginary: bb9];
+        falseEdge -> [real: bb12, imaginary: bb7];
     }
 
     bb5: {
         switchInt((_3.1: bool)) -> [0: bb1, otherwise: bb6];
     }
 
     bb6: {
-        falseEdge -> [real: bb16, imaginary: bb3];
+        falseEdge -> [real: bb16, imaginary: bb1];
     }
 
     bb7: {
@@ -60,7 +60,7 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
     }
 
     bb9: {
-        falseEdge -> [real: bb15, imaginary: bb6];
+        falseEdge -> [real: bb15, imaginary: bb5];
     }
 
     bb10: {
@@ -89,7 +89,7 @@ fn constant_eq(_1: &str, _2: bool) -> u32 {
 
     bb14: {
         StorageDead(_10);
-        falseEdge -> [real: bb5, imaginary: bb9];
+        falseEdge -> [real: bb5, imaginary: bb7];
     }
 
     bb15: {

tests/mir-opt/building/match/sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir

@@ -23,7 +23,7 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 {
     }
 
     bb2: {
-        falseEdge -> [real: bb9, imaginary: bb4];
+        falseEdge -> [real: bb9, imaginary: bb3];
     }
 
     bb3: {
@@ -32,7 +32,7 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 {
     }
 
     bb4: {
-        falseEdge -> [real: bb12, imaginary: bb6];
+        falseEdge -> [real: bb12, imaginary: bb5];
     }
 
     bb5: {
@@ -69,7 +69,7 @@ fn disjoint_ranges(_1: i32, _2: bool) -> u32 {
 
     bb11: {
         StorageDead(_8);
-        falseEdge -> [real: bb1, imaginary: bb4];
+        falseEdge -> [real: bb1, imaginary: bb3];
     }
 
     bb12: {

tests/mir-opt/match_arm_scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff

@@ -60,11 +60,11 @@
       }
 
 -     bb5: {
--         falseEdge -> [real: bb13, imaginary: bb3];
+-         falseEdge -> [real: bb13, imaginary: bb2];
 -     }
 - 
 -     bb6: {
--         falseEdge -> [real: bb8, imaginary: bb5];
+-         falseEdge -> [real: bb8, imaginary: bb1];
 -     }
 - 
 -     bb7: {
@@ -127,7 +127,7 @@
           StorageDead(_9);
           StorageDead(_8);
           StorageDead(_6);
--         falseEdge -> [real: bb1, imaginary: bb5];
+-         falseEdge -> [real: bb1, imaginary: bb1];
 +         goto -> bb1;
       }
 
@@ -184,7 +184,7 @@
           StorageDead(_12);
           StorageDead(_8);
           StorageDead(_6);
--         falseEdge -> [real: bb2, imaginary: bb3];
+-         falseEdge -> [real: bb2, imaginary: bb2];
 +         goto -> bb2;
       }
 

tests/mir-opt/match_arm_scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff

@@ -60,11 +60,11 @@
       }
 
 -     bb5: {
--         falseEdge -> [real: bb13, imaginary: bb3];
+-         falseEdge -> [real: bb13, imaginary: bb2];
 -     }
 - 
 -     bb6: {
--         falseEdge -> [real: bb8, imaginary: bb5];
+-         falseEdge -> [real: bb8, imaginary: bb1];
 -     }
 - 
 -     bb7: {
@@ -127,7 +127,7 @@
           StorageDead(_9);
           StorageDead(_8);
           StorageDead(_6);
--         falseEdge -> [real: bb1, imaginary: bb5];
+-         falseEdge -> [real: bb1, imaginary: bb1];
 +         goto -> bb1;
       }
 
@@ -184,7 +184,7 @@
           StorageDead(_12);
           StorageDead(_8);
           StorageDead(_6);
--         falseEdge -> [real: bb2, imaginary: bb3];
+-         falseEdge -> [real: bb2, imaginary: bb2];
 +         goto -> bb2;
       }