Skip to content

Commit

Permalink
Rollup merge of rust-lang#122221 - Nadrieril:patextradata, r=oli-obk
Browse files Browse the repository at this point in the history
match lowering: define a convenient struct

Small refactor PR: `bindings` and `ascriptions` always come together so I made a struct for them. I'll have one or two fields to add to it in a later PR as well.
  • Loading branch information
matthiaskrgr authored Mar 10, 2024
2 parents 3a5abff + 594cf1d commit 5902d4e
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 72 deletions.
118 changes: 59 additions & 59 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
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,35 @@ 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>>,
}

impl<'tcx> PatternExtraData<'tcx> {
fn is_empty(&self) -> bool {
self.bindings.is_empty() && self.ascriptions.is_empty()
}
}

/// 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 +961,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, all 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 +1011,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 @@ -1518,9 +1524,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.merge_trivial_subcandidates(subcandidate, source_info);

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

if can_merge {
Expand Down Expand Up @@ -1943,7 +1948,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 +1959,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 +1976,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 +2068,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 +2147,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 +2175,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
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
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

0 comments on commit 5902d4e

Please sign in to comment.