user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

lower bindings in the order they're written

Browse source
This commit is contained in:
dianne 2025-07-10 16:38:53 -07:00
parent ea1eca5e3b
commit b7de539805
6 changed files with 128 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

20
compiler/rustc_mir_build/src/builder/matches/match_pair.rs
View file

@ -124,9 +124,19 @@ impl<'tcx> MatchPairTree<'tcx> {
let test_case = match pattern.kind {
PatKind::Missing | PatKind::Wild | PatKind::Error(_) => None,
PatKind::Or { ref pats } => Some(TestCase::Or {
pats: pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect(),
}),
PatKind::Or { ref pats } => {
let pats: Box<[FlatPat<'tcx>]> =
pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect();
if !pats[0].extra_data.bindings.is_empty() {
// Hold a place for any bindings established in (possibly-nested) or-patterns.
// By only holding a place when bindings are present, we skip over any
// or-patterns that will be simplified by `merge_trivial_subcandidates`. In
// other words, we can assume this expands into subcandidates.
// FIXME(@dianne): this needs updating/removing if we always merge or-patterns
extra_data.bindings.push(super::SubpatternBindings::FromOrPattern);
}
Some(TestCase::Or { pats })
}
PatKind::Range(ref range) => {
if range.is_full_range(cx.tcx) == Some(true) {
@ -194,12 +204,12 @@ impl<'tcx> MatchPairTree<'tcx> {
// Then push this binding, after any bindings in the subpattern.
if let Some(source) = place {
extra_data.bindings.push(super::Binding {
extra_data.bindings.push(super::SubpatternBindings::One(super::Binding {
span: pattern.span,
source,
var_id: var,
binding_mode: mode,
});
}));
}
None

82
compiler/rustc_mir_build/src/builder/matches/mod.rs
View file

@ -990,7 +990,7 @@ struct PatternExtraData<'tcx> {
span: Span,
/// Bindings that must be established.
bindings: Vec<Binding<'tcx>>,
bindings: Vec<SubpatternBindings<'tcx>>,
/// Types that must be asserted.
ascriptions: Vec<Ascription<'tcx>>,
@ -1005,6 +1005,15 @@ impl<'tcx> PatternExtraData<'tcx> {
}
}
#[derive(Debug, Clone)]
enum SubpatternBindings<'tcx> {
/// A single binding.
One(Binding<'tcx>),
/// Holds the place for an or-pattern's bindings. This ensures their drops are scheduled in the
/// order the primary bindings appear. See rust-lang/rust#142163 for more information.
FromOrPattern,
}
/// A pattern in a form suitable for lowering the match tree, with all irrefutable
/// patterns simplified away.
///
@ -1220,7 +1229,7 @@ fn traverse_candidate<'tcx, C, T, I>(
}
}
#[derive(Clone, Debug)]
#[derive(Clone, Copy, Debug)]
struct Binding<'tcx> {
span: Span,
source: Place<'tcx>,
@ -1446,12 +1455,7 @@ impl<'tcx> MatchTreeSubBranch<'tcx> {
span: candidate.extra_data.span,
success_block: candidate.pre_binding_block.unwrap(),
otherwise_block: candidate.otherwise_block.unwrap(),
bindings: parent_data
.iter()
.flat_map(|d| &d.bindings)
.chain(&candidate.extra_data.bindings)
.cloned()
.collect(),
bindings: sub_branch_bindings(parent_data, &candidate.extra_data.bindings),
ascriptions: parent_data
.iter()
.flat_map(|d| &d.ascriptions)
@ -1484,6 +1488,68 @@ impl<'tcx> MatchTreeBranch<'tcx> {
}
}
/// Collects the bindings for a [`MatchTreeSubBranch`], preserving the order they appear in the
/// pattern, as though the or-alternatives chosen in this sub-branch were inlined.
fn sub_branch_bindings<'tcx>(
parents: &[PatternExtraData<'tcx>],
leaf_bindings: &[SubpatternBindings<'tcx>],
) -> Vec<Binding<'tcx>> {
// In the common case, all bindings will be in leaves. Allocate to fit the leaf's bindings.
let mut all_bindings = Vec::with_capacity(leaf_bindings.len());
let mut remainder = parents
.iter()
.map(|parent| parent.bindings.as_slice())
.chain([leaf_bindings])
// Skip over unsimplified or-patterns without bindings.
.filter(|bindings| !bindings.is_empty());
if let Some(candidate_bindings) = remainder.next() {
push_sub_branch_bindings(&mut all_bindings, candidate_bindings, &mut remainder);
}
// Make sure we've included all bindings. For ill-formed patterns like `(x, _ | y)`, we may not
// have collected all bindings yet, since we only check the first alternative when determining
// whether to inline subcandidates' bindings.
// FIXME(@dianne): prevent ill-formed patterns from getting here
while let Some(candidate_bindings) = remainder.next() {
ty::tls::with(|tcx| {
tcx.dcx().delayed_bug("mismatched or-pattern bindings but no error emitted")
});
// To recover, we collect the rest in an arbitrary order.
push_sub_branch_bindings(&mut all_bindings, candidate_bindings, &mut remainder);
}
all_bindings
}
/// Helper for [`sub_branch_bindings`]. Collects bindings from `candidate_bindings` into
/// `flattened`. Bindings in or-patterns are collected recursively from `remainder`.
fn push_sub_branch_bindings<'c, 'tcx: 'c>(
flattened: &mut Vec<Binding<'tcx>>,
candidate_bindings: &'c [SubpatternBindings<'tcx>],
remainder: &mut impl Iterator<Item = &'c [SubpatternBindings<'tcx>]>,
) {
for subpat_bindings in candidate_bindings {
match subpat_bindings {
SubpatternBindings::One(binding) => flattened.push(*binding),
SubpatternBindings::FromOrPattern => {
// Inline bindings from an or-pattern. By construction, this always
// corresponds to a subcandidate and its closest descendants (i.e. those
// from nested or-patterns, but not adjacent or-patterns). To handle
// adjacent or-patterns, e.g. `(x | x, y | y)`, we update the `remainder` to
// point to the first descendant candidate from outside this or-pattern.
if let Some(subcandidate_bindings) = remainder.next() {
push_sub_branch_bindings(flattened, subcandidate_bindings, remainder);
} else {
// For ill-formed patterns like `x | _`, we may not have any subcandidates left
// to inline bindings from.
// FIXME(@dianne): prevent ill-formed patterns from getting here
ty::tls::with(|tcx| {
tcx.dcx().delayed_bug("mismatched or-pattern bindings but no error emitted")
});
};
}
}
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum HasMatchGuard {
Yes,

8
compiler/rustc_mir_build/src/builder/matches/util.rs
View file

@ -138,7 +138,9 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> {
fn visit_candidate(&mut self, candidate: &Candidate<'tcx>) {
for binding in &candidate.extra_data.bindings {
self.visit_binding(binding);
if let super::SubpatternBindings::One(binding) = binding {
self.visit_binding(binding);
}
}
for match_pair in &candidate.match_pairs {
self.visit_match_pair(match_pair);
@ -147,7 +149,9 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> {
fn visit_flat_pat(&mut self, flat_pat: &FlatPat<'tcx>) {
for binding in &flat_pat.extra_data.bindings {
self.visit_binding(binding);
if let super::SubpatternBindings::One(binding) = binding {
self.visit_binding(binding);
}
}
for match_pair in &flat_pat.match_pairs {
self.visit_match_pair(match_pair);

45
tests/ui/drop/or-pattern-drop-order.rs
View file

@ -1,6 +1,7 @@
//@ run-pass
//! Test drop order for different ways of declaring pattern bindings involving or-patterns.
//! Currently, it's inconsistent between language constructs (#142163).
//! In particular, are ordered based on the order in which the first occurrence of each binding
//! appears (i.e. the "primary" bindings). Regression test for #142163.
use std::cell::RefCell;
use std::ops::Drop;
@ -43,11 +44,10 @@ fn main() {
y = LogDrop(o, 1);
});
// When bindings are declared with `let pat = expr;`, bindings within or-patterns are seen last,
// thus they're dropped first.
// `let pat = expr;` should have the same drop order.
assert_drop_order(1..=3, |o| {
// Drops are right-to-left, treating `y` as rightmost: `y`, `z`, `x`.
let (x, Ok(y) | Err(y), z) = (LogDrop(o, 3), Ok(LogDrop(o, 1)), LogDrop(o, 2));
// Drops are right-to-left: `z`, `y`, `x`.
let (x, Ok(y) | Err(y), z) = (LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1));
});
assert_drop_order(1..=2, |o| {
// The first or-pattern alternative determines the bindings' drop order: `y`, `x`.
@ -58,11 +58,10 @@ fn main() {
let ((true, x, y) | (false, y, x)) = (false, LogDrop(o, 1), LogDrop(o, 2));
});
// `match` treats or-patterns as last like `let pat = expr;`, but also determines drop order
// using the order of the bindings in the *last* or-pattern alternative.
// `match` should have the same drop order.
assert_drop_order(1..=3, |o| {
// Drops are right-to-left, treating `y` as rightmost: `y`, `z`, `x`.
match (LogDrop(o, 3), Ok(LogDrop(o, 1)), LogDrop(o, 2)) { (x, Ok(y) | Err(y), z) => {} }
// Drops are right-to-left: `z`, `y` `x`.
match (LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1)) { (x, Ok(y) | Err(y), z) => {} }
});
assert_drop_order(1..=2, |o| {
// The first or-pattern alternative determines the bindings' drop order: `y`, `x`.
@ -74,14 +73,14 @@ fn main() {
});
// Function params are visited one-by-one, and the order of bindings within a param's pattern is
// the same as `let pat = expr`;
// the same as `let pat = expr;`
assert_drop_order(1..=3, |o| {
// Among separate params, the drop order is right-to-left: `z`, `y`, `x`.
(|x, (Ok(y) | Err(y)), z| {})(LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1));
});
assert_drop_order(1..=3, |o| {
// Within a param's pattern, or-patterns are treated as rightmost: `y`, `z`, `x`.
(|(x, Ok(y) | Err(y), z)| {})((LogDrop(o, 3), Ok(LogDrop(o, 1)), LogDrop(o, 2)));
// Within a param's pattern, likewise: `z`, `y`, `x`.
(|(x, Ok(y) | Err(y), z)| {})((LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1)));
});
assert_drop_order(1..=2, |o| {
// The first or-pattern alternative determines the bindings' drop order: `y`, `x`.
@ -89,12 +88,11 @@ fn main() {
});
// `if let` and `let`-`else` see bindings in the same order as `let pat = expr;`.
// Vars in or-patterns are seen last (dropped first), and the first alternative's order is used.
assert_drop_order(1..=3, |o| {
if let (x, Ok(y) | Err(y), z) = (LogDrop(o, 3), Ok(LogDrop(o, 1)), LogDrop(o, 2)) {}
if let (x, Ok(y) | Err(y), z) = (LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1)) {}
});
assert_drop_order(1..=3, |o| {
let (x, Ok(y) | Err(y), z) = (LogDrop(o, 3), Ok(LogDrop(o, 1)), LogDrop(o, 2)) else {
let (x, Ok(y) | Err(y), z) = (LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1)) else {
unreachable!();
};
});
@ -106,4 +104,21 @@ fn main() {
unreachable!();
};
});
// Test nested and adjacent or-patterns, including or-patterns without bindings under a guard.
assert_drop_order(1..=6, |o| {
// The `LogDrop`s that aren't moved into bindings are dropped last.
match [
[LogDrop(o, 6), LogDrop(o, 4)],
[LogDrop(o, 3), LogDrop(o, 2)],
[LogDrop(o, 1), LogDrop(o, 5)],
] {
[
[_ | _, w | w] | [w | w, _ | _],
[x | x, y | y] | [y | y, x | x],
[z | z, _ | _] | [_ | _, z | z],
] if true => {}
_ => unreachable!(),
}
});
}

7
tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.rs
View file

@ -1,16 +1,15 @@
//@ known-bug: unknown
//@ run-pass
#![allow(unused)]
struct A(u32);
pub fn main() {
// The or-pattern bindings are lowered after `x`, which triggers the error.
// Bindings are lowered in the order they appear syntactically, so this works.
let x @ (A(a) | A(a)) = A(10);
// ERROR: use of moved value
assert!(x.0 == 10);
assert!(a == 10);
// This works.
// This also works.
let (x @ A(a) | x @ A(a)) = A(10);
assert!(x.0 == 10);
assert!(a == 10);

31
tests/ui/pattern/bindings-after-at/bind-by-copy-or-pat.stderr
View file

@ -1,31 +0,0 @@
error[E0382]: use of moved value
--> $DIR/bind-by-copy-or-pat.rs:8:16
|
LL | let x @ (A(a) | A(a)) = A(10);
| - ^ ----- move occurs because value has type `A`, which does not implement the `Copy` trait
| | |
| | value used here after move
| value moved here
|
help: borrow this binding in the pattern to avoid moving the value
|
LL | let ref x @ (A(a) | A(a)) = A(10);
| +++
error[E0382]: use of moved value
--> $DIR/bind-by-copy-or-pat.rs:8:23
|
LL | let x @ (A(a) | A(a)) = A(10);
| - ^ ----- move occurs because value has type `A`, which does not implement the `Copy` trait
| | |
| | value used here after move
| value moved here
|
help: borrow this binding in the pattern to avoid moving the value
|
LL | let ref x @ (A(a) | A(a)) = A(10);
| +++
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0382`.