Simplify MatcherPos some more
#95509
Conversation
Currently, we detect an exit from a `Delimited` submatcher when `idx` exceeds the bounds of the current submatcher *and* there is a `stack` entry. This commit changes it to something simpler: just look for a `CloseDelim` token.
It's redundant w.r.t. other fields.
By adding comments, and improving an assertion. I finally fully understand this part!
`parse_tt` needs a way to get from within submatchers make to the enclosing submatchers. Currently it has two distinct mechanisms for this: - `Delimited` submatchers use `MatcherPos::stack` to record stuff about the parent (and further back ancestors). - `Sequence` submatchers use `MatcherPosSequence::parent` to point to the parent matcher position. Having two mechanisms is really confusing, and it took me a long time to understand all this. This commit eliminates `MatcherPos::stack`, and changes `Delimited` submatchers to use the same mechanism as sequence submatchers. That mechanism is also changed a bit: instead of storing the entire parent `MatcherPos`, we now only store the necessary parts from the parent `MatcherPos`. Overall this is a small performance win, with the positives outweighing the negatives, but it's mostly for clarity.
It's only used in one place, and there we clone and then make a bunch of modifications. It's clearer if we duplicate more explicitly, and there's a symmetry now between `sequence()` and `empty_sequence()`.
rustbot
added
the
T-compiler
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit c6fedd4 with merge 72313871ade2e188e761c3432e0dbe5b8428258e... |
|
☀️ Try build successful - checks-actions |
|
Queued 72313871ade2e188e761c3432e0dbe5b8428258e with parent a39ac5a, future comparison URL. |
|
Finished benchmarking commit (72313871ade2e188e761c3432e0dbe5b8428258e): comparison url. Summary: This benchmark run shows 11 relevant improvements 🎉 to instruction counts.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
rustbot
removed
the
S-waiting-on-perf
|
@bors r+ |
|
📌 Commit c6fedd4 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (95f6870): comparison url. Summary:
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
A few more improvements.
r? @petrochenkov