Move recursion out of MatchPairTree::for_pattern helpers #154943
Move recursion out of MatchPairTree::for_pattern helpers #154943rust-bors[bot] merged 2 commits intorust-lang:mainfrom
MatchPairTree::for_pattern helpers #154943Conversation
|
Some changes occurred in match lowering cc @Nadrieril |
|
r? @nnethercote rustbot has assigned @nnethercote. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| fn for_each_field_subpat<'tcx>( | ||
| place: &PlaceBuilder<'tcx>, | ||
| subpatterns: &[FieldPat<'tcx>], | ||
| mut callback_fn: impl FnMut(PlaceBuilder<'tcx>, &Pat<'tcx>), |
There was a problem hiding this comment.
Does this need to be a closure? For every occurrence the callback just calls for_pattern. Seems more general than necessary.
| prefix: &[Pat<'tcx>], | ||
| opt_middle: &Option<Box<Pat<'tcx>>>, | ||
| suffix: &[Pat<'tcx>], | ||
| mut callback_fn: impl FnMut(PlaceBuilder<'tcx>, &Pat<'tcx>), |
|
Thinking about this some more, it might be worth just fully inlining both helpers. The field-subpat helper is trivial, and much of the complexity in the array/slice helper comes from generalising over array and slice patterns, which have significantly different indexing needs. |
You can try and see but I think keeping them together is clearer. I'm thinking, how about returning an
True, we could inline that one (Happy to take the r ? on this one, it's code I'm very familiar with) |
|
Ok, thanks! r? @Nadrieril |
|
|
17f0b0a to
4a5c24f
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
I've pushed a brand-new set of commits that:
This is what I'm currently using as a base for #155144, though I think these changes are a reasonable cleanup on their own. @rustbot ready |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Move recursion out of `MatchPairTree::for_pattern` helpers
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (9e54c30): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary 6.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 7.4%, secondary -1.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 504.175s -> 489.73s (-2.87%) |
|
Nice @bors r+ |
|
@bors rollup |
Move recursion out of `MatchPairTree::for_pattern` helpers The helper functions now just iterate over the relevant subpatterns, while leaving recursion up to the main function. This avoids passing parameters that were only used for recursive plumbing, and consolidates all recursive calls into `for_pattern` itself, which should make it easier to experiment with changes to the recursive structure. There should be no change to compiler behaviour.
Rollup of 12 pull requests Successful merges: - #147811 (naked functions: respect `function-sections`) - #154935 (Add Sized supertrait for CoerceUnsized and DispatchFromDyn) - #139690 (`impl Default for RepeatN`) - #153511 (`std::any::TypeId`: remove misplaced "and" in `Unique<T>` example) - #154943 (Move recursion out of `MatchPairTree::for_pattern` helpers ) - #155295 (Fix misleading "borrowed data escapes outside of function" diagnostic) - #155427 (ptr: update text in intro text to one in with_addr doc) - #155428 (Fix ICE in borrowck mutability suggestion with multi-byte ref sigil) - #155435 (rustdoc: Fix `redundant_explicit_links` incorrectly firing (or not firing) under certain scenarios) - #155450 (Remove unnecessary safety conditions related to unchecked uint arithmetic) - #155454 (docs: Fix typo in std/src/thready/scoped.rs) - #155467 (`std::error::Request`: clean up documentation)
Rollup merge of #154943 - Zalathar:for-each-subpat, r=Nadrieril Move recursion out of `MatchPairTree::for_pattern` helpers The helper functions now just iterate over the relevant subpatterns, while leaving recursion up to the main function. This avoids passing parameters that were only used for recursive plumbing, and consolidates all recursive calls into `for_pattern` itself, which should make it easier to experiment with changes to the recursive structure. There should be no change to compiler behaviour.
The helper functions now just iterate over the relevant subpatterns, while leaving recursion up to the main function.
This avoids passing parameters that were only used for recursive plumbing, and consolidates all recursive calls into
for_patternitself, which should make it easier to experiment with changes to the recursive structure.There should be no change to compiler behaviour.