Skip to content

Commit 347cf21

Browse files
committed
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: rust-lang#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.)
1 parent 3efe61c commit 347cf21

File tree

2 files changed

+18
-36
lines changed

2 files changed

+18
-36
lines changed

src/librustc/cfg/construct.rs

+8-34
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,6 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
473473

474474
// Keep track of the previous guard expressions
475475
let mut prev_guards = Vec::new();
476-
// Track if the previous pattern contained bindings or wildcards
477-
let mut prev_has_bindings = false;
478476

479477
for arm in arms {
480478
// Add an exit node for when we've visited all the
@@ -493,40 +491,16 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
493491
// Visit the guard expression
494492
let guard_exit = self.expr(&guard, guard_start);
495493

496-
let this_has_bindings = pat.contains_bindings_or_wild();
497-
498-
// If both this pattern and the previous pattern
499-
// were free of bindings, they must consist only
500-
// of "constant" patterns. Note we cannot match an
501-
// all-constant pattern, fail the guard, and then
502-
// match *another* all-constant pattern. This is
503-
// because if the previous pattern matches, then
504-
// we *cannot* match this one, unless all the
505-
// constants are the same (which is rejected by
506-
// `check_match`).
507-
//
508-
// We can use this to be smarter about the flow
509-
// along guards. If the previous pattern matched,
510-
// then we know we will not visit the guard in
511-
// this one (whether or not the guard succeeded),
512-
// if the previous pattern failed, then we know
513-
// the guard for that pattern will not have been
514-
// visited. Thus, it is not possible to visit both
515-
// the previous guard and the current one when
516-
// both patterns consist only of constant
517-
// sub-patterns.
518-
//
519-
// However, if the above does not hold, then all
520-
// previous guards need to be wired to visit the
521-
// current guard pattern.
522-
if prev_has_bindings || this_has_bindings {
523-
while let Some(prev) = prev_guards.pop() {
524-
self.add_contained_edge(prev, guard_start);
525-
}
494+
// #47295: We used to have very special case code
495+
// here for when a pair of arms are both formed
496+
// solely from constants, and if so, not add these
497+
// edges. But this was not actually sound without
498+
// other constraints that we stopped enforcing at
499+
// some point.
500+
while let Some(prev) = prev_guards.pop() {
501+
self.add_contained_edge(prev, guard_start);
526502
}
527503

528-
prev_has_bindings = this_has_bindings;
529-
530504
// Push the guard onto the list of previous guards
531505
prev_guards.push(guard_exit);
532506

src/test/run-pass/move-guard-const.rs src/test/compile-fail/move-guard-same-consts.rs

+10-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,15 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// pretty-expanded FIXME #23616
11+
// #47295: We used to have a hack of special-casing adjacent amtch
12+
// arms whose patterns were composed solely of constants to not have
13+
// them linked in the cfg.
14+
//
15+
// THis was broken for various reasons. In particular, that hack was
16+
// originally authored under the assunption that other checks
17+
// elsewhere would ensure that the two patterns did not overlap. But
18+
// that assumption did not hold, at least not in the long run (namely,
19+
// overlapping patterns were turned into warnings rather than errors).
1220

1321
#![feature(box_syntax)]
1422

@@ -18,8 +26,8 @@ fn main() {
1826
let v = (1, 2);
1927

2028
match v {
21-
(2, 1) if take(x) => (),
2229
(1, 2) if take(x) => (),
30+
(1, 2) if take(x) => (), //~ ERROR use of moved value: `x`
2331
_ => (),
2432
}
2533
}

0 commit comments

Comments
 (0)