Rollup merge of #81364 - camelid:improve-build-matches-docs, r=varkor

Improve `rustc_mir_build::matches` docs

- Fix typos
- Add more information
- General cleanup
This commit is contained in:
Jonas Schievink 2021-02-01 14:29:31 +01:00 committed by GitHub
commit ab74346b96
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 74 additions and 55 deletions

View File

@ -82,8 +82,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// visible through borrow checking. False edges ensure that the CFG as /// visible through borrow checking. False edges ensure that the CFG as
/// seen by borrow checking doesn't encode this. False edges are added: /// seen by borrow checking doesn't encode this. False edges are added:
/// ///
/// * From each prebinding block to the next prebinding block. /// * From each pre-binding block to the next pre-binding block.
/// * From each otherwise block to the next prebinding block. /// * From each otherwise block to the next pre-binding block.
crate fn match_expr( crate fn match_expr(
&mut self, &mut self,
destination: Place<'tcx>, destination: Place<'tcx>,
@ -630,10 +630,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
#[derive(Debug)] #[derive(Debug)]
pub(super) struct Candidate<'pat, 'tcx> { pub(super) struct Candidate<'pat, 'tcx> {
/// `Span` of the original pattern that gave rise to this candidate /// [`Span`] of the original pattern that gave rise to this candidate.
span: Span, span: Span,
/// This `Candidate` has a guard. /// Whether this `Candidate` has a guard.
has_guard: bool, has_guard: bool,
/// All of these must be satisfied... /// All of these must be satisfied...
@ -648,11 +648,12 @@ pub(super) struct Candidate<'pat, 'tcx> {
/// ...and if this is non-empty, one of these subcandidates also has to match... /// ...and if this is non-empty, one of these subcandidates also has to match...
subcandidates: Vec<Candidate<'pat, 'tcx>>, subcandidates: Vec<Candidate<'pat, 'tcx>>,
/// ...and the guard must be evaluated, if false branch to Block... /// ...and the guard must be evaluated; if it's `false` then branch to `otherwise_block`.
otherwise_block: Option<BasicBlock>, otherwise_block: Option<BasicBlock>,
/// ...and the blocks for add false edges between candidates /// The block before the `bindings` have been established.
pre_binding_block: Option<BasicBlock>, pre_binding_block: Option<BasicBlock>,
/// The pre-binding block of the next candidate.
next_candidate_pre_binding_block: Option<BasicBlock>, next_candidate_pre_binding_block: Option<BasicBlock>,
} }
@ -737,18 +738,19 @@ crate struct MatchPair<'pat, 'tcx> {
pattern: &'pat Pat<'tcx>, pattern: &'pat Pat<'tcx>,
} }
/// See [`Test`] for more.
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, Debug, PartialEq)]
enum TestKind<'tcx> { enum TestKind<'tcx> {
/// Test the branches of enum. /// Test what enum variant a value is.
Switch { Switch {
/// The enum being tested /// The enum type being tested.
adt_def: &'tcx ty::AdtDef, adt_def: &'tcx ty::AdtDef,
/// The set of variants that we should create a branch for. We also /// The set of variants that we should create a branch for. We also
/// create an additional "otherwise" case. /// create an additional "otherwise" case.
variants: BitSet<VariantIdx>, variants: BitSet<VariantIdx>,
}, },
/// Test what value an `integer`, `bool` or `char` has. /// Test what value an integer, `bool`, or `char` has.
SwitchInt { SwitchInt {
/// The type of the value that we're testing. /// The type of the value that we're testing.
switch_ty: Ty<'tcx>, switch_ty: Ty<'tcx>,
@ -756,7 +758,7 @@ enum TestKind<'tcx> {
/// ///
/// For integers and `char`s we create a branch to each of the values in /// For integers and `char`s we create a branch to each of the values in
/// `options`, as well as an "otherwise" branch for all other values, even /// `options`, as well as an "otherwise" branch for all other values, even
/// in the (rare) case that options is exhaustive. /// in the (rare) case that `options` is exhaustive.
/// ///
/// For `bool` we always generate two edges, one for `true` and one for /// For `bool` we always generate two edges, one for `true` and one for
/// `false`. /// `false`.
@ -776,17 +778,21 @@ enum TestKind<'tcx> {
/// Test whether the value falls within an inclusive or exclusive range /// Test whether the value falls within an inclusive or exclusive range
Range(PatRange<'tcx>), Range(PatRange<'tcx>),
/// Test length of the slice is equal to len /// Test that the length of the slice is equal to `len`.
Len { len: u64, op: BinOp }, Len { len: u64, op: BinOp },
} }
/// A test to perform to determine which [`Candidate`] matches a value.
///
/// [`Test`] is just the test to perform; it does not include the value
/// to be tested.
#[derive(Debug)] #[derive(Debug)]
crate struct Test<'tcx> { crate struct Test<'tcx> {
span: Span, span: Span,
kind: TestKind<'tcx>, kind: TestKind<'tcx>,
} }
/// ArmHasGuard is isomorphic to a boolean flag. It indicates whether /// `ArmHasGuard` is a wrapper around a boolean flag. It indicates whether
/// a match arm has a guard expression attached to it. /// a match arm has a guard expression attached to it.
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug)]
crate struct ArmHasGuard(crate bool); crate struct ArmHasGuard(crate bool);
@ -801,27 +807,27 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// candidates are sorted such that the first item in the list /// candidates are sorted such that the first item in the list
/// has the highest priority. When a candidate is found to match /// has the highest priority. When a candidate is found to match
/// the value, we will set and generate a branch to the appropriate /// the value, we will set and generate a branch to the appropriate
/// prebinding block. /// pre-binding block.
/// ///
/// If we find that *NONE* of the candidates apply, we branch to the /// If we find that *NONE* of the candidates apply, we branch to the
/// `otherwise_block`, setting it to `Some` if required. In principle, this /// `otherwise_block`, setting it to `Some` if required. In principle, this
/// means that the input list was not exhaustive, though at present we /// means that the input list was not exhaustive, though at present we
/// sometimes are not smart enough to recognize all exhaustive inputs. /// sometimes are not smart enough to recognize all exhaustive inputs.
/// ///
/// It might be surprising that the input can be inexhaustive. /// It might be surprising that the input can be non-exhaustive.
/// Indeed, initially, it is not, because all matches are /// Indeed, initially, it is not, because all matches are
/// exhaustive in Rust. But during processing we sometimes divide /// exhaustive in Rust. But during processing we sometimes divide
/// up the list of candidates and recurse with a non-exhaustive /// up the list of candidates and recurse with a non-exhaustive
/// list. This is important to keep the size of the generated code /// list. This is important to keep the size of the generated code
/// under control. See `test_candidates` for more details. /// under control. See [`Builder::test_candidates`] for more details.
/// ///
/// If `fake_borrows` is Some, then places which need fake borrows /// If `fake_borrows` is `Some`, then places which need fake borrows
/// will be added to it. /// will be added to it.
/// ///
/// For an example of a case where we set `otherwise_block`, even for an /// For an example of a case where we set `otherwise_block`, even for an
/// exhaustive match consider: /// exhaustive match, consider:
/// ///
/// ```rust /// ```
/// match x { /// match x {
/// (true, true) => (), /// (true, true) => (),
/// (_, false) => (), /// (_, false) => (),
@ -830,8 +836,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// ``` /// ```
/// ///
/// For this match, we check if `x.0` matches `true` (for the first /// For this match, we check if `x.0` matches `true` (for the first
/// arm). If that's false, we check `x.1`. If it's `true` we check if /// arm). If it doesn't match, we check `x.1`. If `x.1` is `true` we check
/// `x.0` matches `false` (for the third arm). In the (impossible at /// if `x.0` matches `false` (for the third arm). In the (impossible at
/// runtime) case when `x.0` is now `true`, we branch to /// runtime) case when `x.0` is now `true`, we branch to
/// `otherwise_block`. /// `otherwise_block`.
fn match_candidates<'pat>( fn match_candidates<'pat>(
@ -938,26 +944,31 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
); );
} }
/// Link up matched candidates. For example, if we have something like /// Link up matched candidates.
/// this: ///
/// For example, if we have something like this:
/// ///
/// ```rust /// ```rust
/// ... /// ...
/// Some(x) if cond => ... /// Some(x) if cond1 => ...
/// Some(x) => ... /// Some(x) => ...
/// Some(x) if cond => ... /// Some(x) if cond2 => ...
/// ... /// ...
/// ``` /// ```
/// ///
/// We generate real edges from: /// We generate real edges from:
/// * `start_block` to the `prebinding_block` of the first pattern,
/// * the otherwise block of the first pattern to the second pattern,
/// * the otherwise block of the third pattern to the a block with an
/// Unreachable terminator.
/// ///
/// As well as that we add fake edges from the otherwise blocks to the /// * `start_block` to the [pre-binding block] of the first pattern,
/// prebinding block of the next candidate in the original set of /// * the [otherwise block] of the first pattern to the second pattern,
/// * the [otherwise block] of the third pattern to a block with an
/// [`Unreachable` terminator](TerminatorKind::Unreachable).
///
/// In addition, we add fake edges from the otherwise blocks to the
/// pre-binding block of the next candidate in the original set of
/// candidates. /// candidates.
///
/// [pre-binding block]: Candidate::pre_binding_block
/// [otherwise block]: Candidate::otherwise_block
fn select_matched_candidates( fn select_matched_candidates(
&mut self, &mut self,
matched_candidates: &mut [&mut Candidate<'_, 'tcx>], matched_candidates: &mut [&mut Candidate<'_, 'tcx>],
@ -1044,7 +1055,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// forwards to [Builder::test_candidates]. /// forwards to [Builder::test_candidates].
/// ///
/// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like /// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like
/// so /// so:
/// ///
/// ```text /// ```text
/// [ start ] /// [ start ]
@ -1214,10 +1225,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// This is the most subtle part of the matching algorithm. At /// This is the most subtle part of the matching algorithm. At
/// this point, the input candidates have been fully simplified, /// this point, the input candidates have been fully simplified,
/// and so we know that all remaining match-pairs require some /// and so we know that all remaining match-pairs require some
/// sort of test. To decide what test to do, we take the highest /// sort of test. To decide what test to perform, we take the highest
/// priority candidate (last one in the list) and extract the /// priority candidate (the first one in the list, as of January 2021)
/// first match-pair from the list. From this we decide what kind /// and extract the first match-pair from the list. From this we decide
/// of test is needed using `test`, defined in the `test` module. /// what kind of test is needed using [`Builder::test`], defined in the
/// [`test` module](mod@test).
/// ///
/// *Note:* taking the first match pair is somewhat arbitrary, and /// *Note:* taking the first match pair is somewhat arbitrary, and
/// we might do better here by choosing more carefully what to /// we might do better here by choosing more carefully what to
@ -1225,20 +1237,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// ///
/// For example, consider the following possible match-pairs: /// For example, consider the following possible match-pairs:
/// ///
/// 1. `x @ Some(P)` -- we will do a `Switch` to decide what variant `x` has /// 1. `x @ Some(P)` -- we will do a [`Switch`] to decide what variant `x` has
/// 2. `x @ 22` -- we will do a `SwitchInt` /// 2. `x @ 22` -- we will do a [`SwitchInt`] to decide what value `x` has
/// 3. `x @ 3..5` -- we will do a range test /// 3. `x @ 3..5` -- we will do a [`Range`] test to decide what range `x` falls in
/// 4. etc. /// 4. etc.
/// ///
/// [`Switch`]: TestKind::Switch
/// [`SwitchInt`]: TestKind::SwitchInt
/// [`Range`]: TestKind::Range
///
/// Once we know what sort of test we are going to perform, this /// Once we know what sort of test we are going to perform, this
/// Tests may also help us with other candidates. So we walk over /// test may also help us winnow down our candidates. So we walk over
/// the candidates (from high to low priority) and check. This /// the candidates (from high to low priority) and check. This
/// gives us, for each outcome of the test, a transformed list of /// gives us, for each outcome of the test, a transformed list of
/// candidates. For example, if we are testing the current /// candidates. For example, if we are testing `x.0`'s variant,
/// variant of `x.0`, and we have a candidate `{x.0 @ Some(v), x.1 /// and we have a candidate `(x.0 @ Some(v), x.1 @ 22)`,
/// @ 22}`, then we would have a resulting candidate of `{(x.0 as /// then we would have a resulting candidate of `((x.0 as Some).0 @ v, x.1 @ 22)`.
/// Some).0 @ v, x.1 @ 22}`. Note that the first match-pair is now /// Note that the first match-pair is now simpler (and, in fact, irrefutable).
/// simpler (and, in fact, irrefutable).
/// ///
/// But there may also be candidates that the test just doesn't /// But there may also be candidates that the test just doesn't
/// apply to. The classical example involves wildcards: /// apply to. The classical example involves wildcards:
@ -1268,7 +1283,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// is trivially NP-complete: /// is trivially NP-complete:
/// ///
/// ```rust /// ```rust
/// match (var0, var1, var2, var3, ..) { /// match (var0, var1, var2, var3, ...) {
/// (true, _, _, false, true, ...) => false, /// (true, _, _, false, true, ...) => false,
/// (_, true, true, false, _, ...) => false, /// (_, true, true, false, _, ...) => false,
/// (false, _, false, false, _, ...) => false, /// (false, _, false, false, _, ...) => false,
@ -1283,7 +1298,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// ///
/// That kind of exponential worst-case might not occur in practice, but /// That kind of exponential worst-case might not occur in practice, but
/// our simplistic treatment of constants and guards would make it occur /// our simplistic treatment of constants and guards would make it occur
/// in very common situations - for example #29740: /// in very common situations - for example [#29740]:
/// ///
/// ```rust /// ```rust
/// match x { /// match x {
@ -1294,13 +1309,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// } /// }
/// ``` /// ```
/// ///
/// Here we first test the match-pair `x @ "foo"`, which is an `Eq` test. /// [#29740]: https://github.com/rust-lang/rust/issues/29740
///
/// Here we first test the match-pair `x @ "foo"`, which is an [`Eq` test].
///
/// [`Eq` test]: TestKind::Eq
/// ///
/// It might seem that we would end up with 2 disjoint candidate /// It might seem that we would end up with 2 disjoint candidate
/// sets, consisting of the first candidate or the other 3, but our /// sets, consisting of the first candidate or the other two, but our
/// algorithm doesn't reason about "foo" being distinct from the other /// algorithm doesn't reason about `"foo"` being distinct from the other
/// constants; it considers the latter arms to potentially match after /// constants; it considers the latter arms to potentially match after
/// both outcomes, which obviously leads to an exponential amount /// both outcomes, which obviously leads to an exponential number
/// of tests. /// of tests.
/// ///
/// To avoid these kinds of problems, our algorithm tries to ensure /// To avoid these kinds of problems, our algorithm tries to ensure
@ -1312,16 +1331,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// ///
/// After we perform our test, we branch into the appropriate candidate /// After we perform our test, we branch into the appropriate candidate
/// set and recurse with `match_candidates`. These sub-matches are /// set and recurse with `match_candidates`. These sub-matches are
/// obviously inexhaustive - as we discarded our otherwise set - so /// obviously non-exhaustive - as we discarded our otherwise set - so
/// we set their continuation to do `match_candidates` on the /// we set their continuation to do `match_candidates` on the
/// "unmatched" set (which is again inexhaustive). /// "unmatched" set (which is again non-exhaustive).
/// ///
/// If you apply this to the above test, you basically wind up /// If you apply this to the above test, you basically wind up
/// with an if-else-if chain, testing each candidate in turn, /// with an if-else-if chain, testing each candidate in turn,
/// which is precisely what we want. /// which is precisely what we want.
/// ///
/// In addition to avoiding exponential-time blowups, this algorithm /// In addition to avoiding exponential-time blowups, this algorithm
/// also has nice property that each guard and arm is only generated /// also has the nice property that each guard and arm is only generated
/// once. /// once.
fn test_candidates<'pat, 'b, 'c>( fn test_candidates<'pat, 'b, 'c>(
&mut self, &mut self,

View File

@ -23,7 +23,7 @@ use std::cmp::Ordering;
impl<'a, 'tcx> Builder<'a, 'tcx> { impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Identifies what test is needed to decide if `match_pair` is applicable. /// Identifies what test is needed to decide if `match_pair` is applicable.
/// ///
/// It is a bug to call this with a simplifiable pattern. /// It is a bug to call this with a not-fully-simplified pattern.
pub(super) fn test<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> Test<'tcx> { pub(super) fn test<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> Test<'tcx> {
match *match_pair.pattern.kind { match *match_pair.pattern.kind {
PatKind::Variant { ref adt_def, substs: _, variant_index: _, subpatterns: _ } => Test { PatKind::Variant { ref adt_def, substs: _, variant_index: _, subpatterns: _ } => Test {