Auto merge of #49447 - pnkfelix:remove-cfg-const-pat-hack-47295, r=nikomatsakis

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.)
This commit is contained in:
bors 2018-04-03 11:50:11 +00:00
commit 637ac17c52
2 changed files with 18 additions and 36 deletions

View File

@ -473,8 +473,6 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
// Keep track of the previous guard expressions // Keep track of the previous guard expressions
let mut prev_guards = Vec::new(); 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 { for arm in arms {
// Add an exit node for when we've visited all the // 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 // Visit the guard expression
let guard_exit = self.expr(&guard, guard_start); let guard_exit = self.expr(&guard, guard_start);
let this_has_bindings = pat.contains_bindings_or_wild(); // #47295: We used to have very special case code
// here for when a pair of arms are both formed
// If both this pattern and the previous pattern // solely from constants, and if so, not add these
// were free of bindings, they must consist only // edges. But this was not actually sound without
// of "constant" patterns. Note we cannot match an // other constraints that we stopped enforcing at
// all-constant pattern, fail the guard, and then // some point.
// match *another* all-constant pattern. This is while let Some(prev) = prev_guards.pop() {
// because if the previous pattern matches, then self.add_contained_edge(prev, guard_start);
// 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);
}
} }
prev_has_bindings = this_has_bindings;
// Push the guard onto the list of previous guards // Push the guard onto the list of previous guards
prev_guards.push(guard_exit); prev_guards.push(guard_exit);

View File

@ -8,7 +8,15 @@
// option. This file may not be copied, modified, or distributed // option. This file may not be copied, modified, or distributed
// except according to those terms. // 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)] #![feature(box_syntax)]
@ -18,8 +26,8 @@ fn main() {
let v = (1, 2); let v = (1, 2);
match v { match v {
(2, 1) if take(x) => (),
(1, 2) if take(x) => (), (1, 2) if take(x) => (),
(1, 2) if take(x) => (), //~ ERROR use of moved value: `x`
_ => (), _ => (),
} }
} }