Skip to content

Commit

Permalink
Auto merge of #49447 - pnkfelix:remove-cfg-const-pat-hack-47295, r=ni…
Browse files Browse the repository at this point in the history
…komatsakis

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:
#47295 (comment)

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.)
  • Loading branch information
bors committed Apr 3, 2018
2 parents b12af86 + 347cf21 commit 637ac17
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 36 deletions.
42 changes: 8 additions & 34 deletions src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

Expand All @@ -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`
_ => (),
}
}
Expand Down

0 comments on commit 637ac17

Please sign in to comment.