From 347cf21290c80c528363cc33d9cade6cd1e77888 Mon Sep 17 00:00:00 2001
From: "Felix S. Klock II" <pnkfelix@pnkfx.org>
Date: Wed, 28 Mar 2018 12:03:28 +0200
Subject: [PATCH] Remove adjacent all-const match arm hack.

An old fix for moves-in-guards had a hack for adjacent all-const match arms.

The hack was explained in a comment, which you can see here:
https://github.com/rust-lang/rust/pull/22580/files#diff-402a0fa4b3c6755c5650027c6d4cf1efR497

But hack was incomplete (and thus unsound), as pointed out here:
https://github.com/rust-lang/rust/issues/47295#issuecomment-357108458

Plus, it is likely to be at least tricky to reimplement this hack in
the new NLL borrowck.

So rather than try to preserve the hack, we want to try to just remove
it outright. (At least to see the results of a crater run.)

[breaking-change]

This is a breaking-change, but our hope is that no one is actually
relying on such an extreme special case. (We hypothesize the hack was
originally added to accommodate a file in our own test suite, not code
in the wild.)
---
 src/librustc/cfg/construct.rs                 | 42 ++++---------------
 .../move-guard-same-consts.rs}                | 12 +++++-
 2 files changed, 18 insertions(+), 36 deletions(-)
 rename src/test/{run-pass/move-guard-const.rs => compile-fail/move-guard-same-consts.rs} (53%)

diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs
index ff2c36416bf..1247db55f58 100644
--- a/src/librustc/cfg/construct.rs
+++ b/src/librustc/cfg/construct.rs
@@ -473,8 +473,6 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
 
         // Keep track of the previous guard expressions
         let mut prev_guards = Vec::new();
-        // Track if the previous pattern contained bindings or wildcards
-        let mut prev_has_bindings = false;
 
         for arm in arms {
             // Add an exit node for when we've visited all the
@@ -493,40 +491,16 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
                     // Visit the guard expression
                     let guard_exit = self.expr(&guard, guard_start);
 
-                    let this_has_bindings = pat.contains_bindings_or_wild();
-
-                    // If both this pattern and the previous pattern
-                    // were free of bindings, they must consist only
-                    // of "constant" patterns. Note we cannot match an
-                    // all-constant pattern, fail the guard, and then
-                    // match *another* all-constant pattern. This is
-                    // because if the previous pattern matches, then
-                    // we *cannot* match this one, unless all the
-                    // constants are the same (which is rejected by
-                    // `check_match`).
-                    //
-                    // We can use this to be smarter about the flow
-                    // along guards. If the previous pattern matched,
-                    // then we know we will not visit the guard in
-                    // this one (whether or not the guard succeeded),
-                    // if the previous pattern failed, then we know
-                    // the guard for that pattern will not have been
-                    // visited. Thus, it is not possible to visit both
-                    // the previous guard and the current one when
-                    // both patterns consist only of constant
-                    // sub-patterns.
-                    //
-                    // However, if the above does not hold, then all
-                    // previous guards need to be wired to visit the
-                    // current guard pattern.
-                    if prev_has_bindings || this_has_bindings {
-                        while let Some(prev) = prev_guards.pop() {
-                            self.add_contained_edge(prev, guard_start);
-                        }
+                    // #47295: We used to have very special case code
+                    // here for when a pair of arms are both formed
+                    // solely from constants, and if so, not add these
+                    // edges.  But this was not actually sound without
+                    // other constraints that we stopped enforcing at
+                    // some point.
+                    while let Some(prev) = prev_guards.pop() {
+                        self.add_contained_edge(prev, guard_start);
                     }
 
-                    prev_has_bindings = this_has_bindings;
-
                     // Push the guard onto the list of previous guards
                     prev_guards.push(guard_exit);
 
diff --git a/src/test/run-pass/move-guard-const.rs b/src/test/compile-fail/move-guard-same-consts.rs
similarity index 53%
rename from src/test/run-pass/move-guard-const.rs
rename to src/test/compile-fail/move-guard-same-consts.rs
index 6e49538e98a..05fe48e0199 100644
--- a/src/test/run-pass/move-guard-const.rs
+++ b/src/test/compile-fail/move-guard-same-consts.rs
@@ -8,7 +8,15 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// pretty-expanded FIXME #23616
+// #47295: We used to have a hack of special-casing adjacent amtch
+// arms whose patterns were composed solely of constants to not have
+// them linked in the cfg.
+//
+// THis was broken for various reasons. In particular, that hack was
+// originally authored under the assunption that other checks
+// elsewhere would ensure that the two patterns did not overlap.  But
+// that assumption did not hold, at least not in the long run (namely,
+// overlapping patterns were turned into warnings rather than errors).
 
 #![feature(box_syntax)]
 
@@ -18,8 +26,8 @@ fn main() {
     let v = (1, 2);
 
     match v {
-        (2, 1) if take(x) => (),
         (1, 2) if take(x) => (),
+        (1, 2) if take(x) => (), //~ ERROR use of moved value: `x`
         _ => (),
     }
 }