Skip to content

match lowering: define a convenient struct #122221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 10, 2024
Merged

match lowering: define a convenient struct #122221

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7843e46
Factor out non-branch-related pattern data
Nadrieril Mar 9, 2024
594cf1d
review
Nadrieril Mar 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 53 additions & 58 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,13 +506,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
traverse_candidate(
candidate,
&mut Vec::new(),
&mut |leaf_candidate, parent_bindings| {
&mut |leaf_candidate, parent_data| {
if let Some(arm) = arm {
self.clear_top_scope(arm.scope);
}
let binding_end = self.bind_and_guard_matched_candidate(
leaf_candidate,
parent_bindings,
parent_data,
fake_borrow_temps,
scrutinee_span,
arm_match_scope,
Expand All @@ -524,12 +524,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
self.cfg.goto(binding_end, outer_source_info, target_block);
},
|inner_candidate, parent_bindings| {
parent_bindings.push((inner_candidate.bindings, inner_candidate.ascriptions));
|inner_candidate, parent_data| {
parent_data.push(inner_candidate.extra_data);
inner_candidate.subcandidates.into_iter()
},
|parent_bindings| {
parent_bindings.pop();
|parent_data| {
parent_data.pop();
},
);

Expand Down Expand Up @@ -651,7 +651,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if set_match_place {
let mut next = Some(&candidate);
while let Some(candidate_ref) = next.take() {
for binding in &candidate_ref.bindings {
for binding in &candidate_ref.extra_data.bindings {
let local = self.var_local_id(binding.var_id, OutsideGuard);
// `try_to_place` may fail if it is unable to resolve the given
// `PlaceBuilder` inside a closure. In this case, we don't want to include
Expand Down Expand Up @@ -924,22 +924,29 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

/// A pattern in a form suitable for generating code.
/// Data extracted from a pattern that doesn't affect which branch is taken. Collected during
/// pattern simplification and not mutated later.
#[derive(Debug, Clone)]
struct FlatPat<'pat, 'tcx> {
struct PatternExtraData<'tcx> {
/// [`Span`] of the original pattern.
span: Span,

/// Bindings that must be established.
bindings: Vec<Binding<'tcx>>,

/// Types that must be asserted.
ascriptions: Vec<Ascription<'tcx>>,
}

/// A pattern in a form suitable for generating code.
#[derive(Debug, Clone)]
struct FlatPat<'pat, 'tcx> {
/// To match the pattern, all of these must be satisfied...
// Invariant: all the `MatchPair`s are recursively simplified.
// Invariant: or-patterns must be sorted to the end.
match_pairs: Vec<MatchPair<'pat, 'tcx>>,

/// ...these bindings established...
bindings: Vec<Binding<'tcx>>,

/// ...and these types asserted.
ascriptions: Vec<Ascription<'tcx>>,
extra_data: PatternExtraData<'tcx>,
}

impl<'tcx, 'pat> FlatPat<'pat, 'tcx> {
Expand All @@ -948,43 +955,38 @@ impl<'tcx, 'pat> FlatPat<'pat, 'tcx> {
pattern: &'pat Pat<'tcx>,
cx: &mut Builder<'_, 'tcx>,
) -> Self {
let mut match_pairs = vec![MatchPair::new(place, pattern, cx)];
let mut bindings = Vec::new();
let mut ascriptions = Vec::new();

cx.simplify_match_pairs(&mut match_pairs, &mut bindings, &mut ascriptions);

FlatPat { span: pattern.span, match_pairs, bindings, ascriptions }
let mut flat_pat = FlatPat {
match_pairs: vec![MatchPair::new(place, pattern, cx)],
extra_data: PatternExtraData {
span: pattern.span,
bindings: Vec::new(),
ascriptions: Vec::new(),
},
};
cx.simplify_match_pairs(&mut flat_pat.match_pairs, &mut flat_pat.extra_data);
flat_pat
}
}

#[derive(Debug)]
struct Candidate<'pat, 'tcx> {
/// [`Span`] of the original pattern that gave rise to this candidate.
span: Span,

/// Whether this `Candidate` has a guard.
has_guard: bool,

/// All of these must be satisfied...
/// For the candidate to match, &ll of these must be satisfied...
// Invariant: all the `MatchPair`s are recursively simplified.
// Invariant: or-patterns must be sorted at the end.
match_pairs: Vec<MatchPair<'pat, 'tcx>>,

/// ...these bindings established...
// Invariant: not mutated after candidate creation.
bindings: Vec<Binding<'tcx>>,

/// ...and these types asserted...
// Invariant: not mutated after candidate creation.
ascriptions: Vec<Ascription<'tcx>>,

/// ...and if this is non-empty, one of these subcandidates also has to match...
subcandidates: Vec<Candidate<'pat, 'tcx>>,

/// ...and the guard must be evaluated; if it's `false` then branch to `otherwise_block`.
/// ...and the guard must be evaluated if there is one.
has_guard: bool,

/// If the guard is `false` then branch to `otherwise_block`.
otherwise_block: Option<BasicBlock>,

/// If the candidate matches, bindings and ascriptions must be established.
extra_data: PatternExtraData<'tcx>,

/// The block before the `bindings` have been established.
pre_binding_block: Option<BasicBlock>,
/// The pre-binding block of the next candidate.
Expand All @@ -1003,10 +1005,8 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {

fn from_flat_pat(flat_pat: FlatPat<'pat, 'tcx>, has_guard: bool) -> Self {
Candidate {
span: flat_pat.span,
match_pairs: flat_pat.match_pairs,
bindings: flat_pat.bindings,
ascriptions: flat_pat.ascriptions,
extra_data: flat_pat.extra_data,
has_guard,
subcandidates: Vec::new(),
otherwise_block: None,
Expand Down Expand Up @@ -1519,8 +1519,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// FIXME(or_patterns; matthewjasper) Try to be more aggressive here.
can_merge &= subcandidate.subcandidates.is_empty()
&& subcandidate.bindings.is_empty()
&& subcandidate.ascriptions.is_empty();
&& subcandidate.extra_data.bindings.is_empty()
&& subcandidate.extra_data.ascriptions.is_empty();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the future information you add here be similar to these two lists and checked here? If so, maybe a single is_empty method on the struct could be useful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good shout 👍

}

if can_merge {
Expand Down Expand Up @@ -1943,7 +1943,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn bind_and_guard_matched_candidate<'pat>(
&mut self,
candidate: Candidate<'pat, 'tcx>,
parent_bindings: &[(Vec<Binding<'tcx>>, Vec<Ascription<'tcx>>)],
parent_data: &[PatternExtraData<'tcx>],
fake_borrows: &[(Place<'tcx>, Local)],
scrutinee_span: Span,
arm_match_scope: Option<(&Arm<'tcx>, region::Scope)>,
Expand All @@ -1954,7 +1954,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

debug_assert!(candidate.match_pairs.is_empty());

let candidate_source_info = self.source_info(candidate.span);
let candidate_source_info = self.source_info(candidate.extra_data.span);

let mut block = candidate.pre_binding_block.unwrap();

Expand All @@ -1971,11 +1971,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

self.ascribe_types(
block,
parent_bindings
parent_data
.iter()
.flat_map(|(_, ascriptions)| ascriptions)
.flat_map(|d| &d.ascriptions)
.cloned()
.chain(candidate.ascriptions),
.chain(candidate.extra_data.ascriptions),
);

// rust-lang/rust#27282: The `autoref` business deserves some
Expand Down Expand Up @@ -2063,10 +2063,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&& let Some(guard) = arm.guard
{
let tcx = self.tcx;
let bindings = parent_bindings
.iter()
.flat_map(|(bindings, _)| bindings)
.chain(&candidate.bindings);
let bindings =
parent_data.iter().flat_map(|d| &d.bindings).chain(&candidate.extra_data.bindings);

self.bind_matched_candidate_for_guard(block, schedule_drops, bindings.clone());
let guard_frame = GuardFrame {
Expand Down Expand Up @@ -2144,10 +2142,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// ```
//
// and that is clearly not correct.
let by_value_bindings = parent_bindings
let by_value_bindings = parent_data
.iter()
.flat_map(|(bindings, _)| bindings)
.chain(&candidate.bindings)
.flat_map(|d| &d.bindings)
.chain(&candidate.extra_data.bindings)
.filter(|binding| matches!(binding.binding_mode, BindingMode::ByValue));
// Read all of the by reference bindings to ensure that the
// place they refer to can't be modified by the guard.
Expand All @@ -2172,10 +2170,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.bind_matched_candidate_for_arm_body(
block,
schedule_drops,
parent_bindings
.iter()
.flat_map(|(bindings, _)| bindings)
.chain(&candidate.bindings),
parent_data.iter().flat_map(|d| &d.bindings).chain(&candidate.extra_data.bindings),
storages_alive,
);
block
Expand Down
17 changes: 6 additions & 11 deletions compiler/rustc_mir_build/src/build/matches/simplify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,19 @@
//! sort of test: for example, testing which variant an enum is, or
//! testing a value against a constant.

use crate::build::matches::{Ascription, Binding, Candidate, FlatPat, MatchPair, TestCase};
use crate::build::matches::{Candidate, FlatPat, MatchPair, PatternExtraData, TestCase};
use crate::build::Builder;

use std::mem;

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Simplify a list of match pairs so they all require a test. Stores relevant bindings and
/// ascriptions in the provided `Vec`s.
/// ascriptions in `extra_data`.
#[instrument(skip(self), level = "debug")]
pub(super) fn simplify_match_pairs<'pat>(
&mut self,
match_pairs: &mut Vec<MatchPair<'pat, 'tcx>>,
candidate_bindings: &mut Vec<Binding<'tcx>>,
candidate_ascriptions: &mut Vec<Ascription<'tcx>>,
extra_data: &mut PatternExtraData<'tcx>,
) {
// In order to please the borrow checker, in a pattern like `x @ pat` we must lower the
// bindings in `pat` before `x`. E.g. (#69971):
Expand All @@ -45,17 +44,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// after any bindings in `pat`. This doesn't work for or-patterns: the current structure of
// match lowering forces us to lower bindings inside or-patterns last.
for mut match_pair in mem::take(match_pairs) {
self.simplify_match_pairs(
&mut match_pair.subpairs,
candidate_bindings,
candidate_ascriptions,
);
self.simplify_match_pairs(&mut match_pair.subpairs, extra_data);
if let TestCase::Irrefutable { binding, ascription } = match_pair.test_case {
if let Some(binding) = binding {
candidate_bindings.push(binding);
extra_data.bindings.push(binding);
}
if let Some(ascription) = ascription {
candidate_ascriptions.push(ascription);
extra_data.ascriptions.push(ascription);
}
// Simplifiable pattern; we replace it with its already simplified subpairs.
match_pairs.append(&mut match_pair.subpairs);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/build/matches/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> {
}

fn visit_candidate(&mut self, candidate: &Candidate<'_, 'tcx>) {
for binding in &candidate.bindings {
for binding in &candidate.extra_data.bindings {
self.visit_binding(binding);
}
for match_pair in &candidate.match_pairs {
Expand All @@ -289,7 +289,7 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> {
}

fn visit_flat_pat(&mut self, flat_pat: &FlatPat<'_, 'tcx>) {
for binding in &flat_pat.bindings {
for binding in &flat_pat.extra_data.bindings {
self.visit_binding(binding);
}
for match_pair in &flat_pat.match_pairs {
Expand Down