-
Notifications
You must be signed in to change notification settings - Fork 13.7k
coverage: Build an "expansion tree" and use it to unexpand raw spans #145643
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.
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
base: master
Are you sure you want to change the base?
Conversation
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
(Note that we currently don't have any benchmarks for |
@@ -173,7 +173,7 @@ | |||
LL| | | |||
LL| 1| let _short_unused_closure_line_break_no_block2 = | |||
LL| | | _unused_arg: u8 | | |||
LL| | println!( | |||
LL| 0| println!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, the span for this println!(..)
was discarded early, because it covers the whole body span of its surrounding closure. In the new implementation, the span gets truncated to just the println!
part before the body-span check, so it is retained, giving slightly better instrumentation.
This wasn't an intentional improvement, but I'll take it.
This comment was marked as resolved.
This comment was marked as resolved.
dac2d18
to
770abe7
Compare
This comment has been minimized.
This comment has been minimized.
This PR was rebased onto a different master 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. |
Historically and currently, coverage instrumentation assumes that all of a function's spans are in the same file and have the same syntax context. The spans extracted directly from MIR don't satisfy that assumption, so there is an “unexpansion” step that walks up each span's expansion-call-site tree to find a suitable span in the same context as the function's body span.
(That unexpansion step is what allows us to have somewhat reasonable coverage instrumentation for macros like
println!
, and for syntax likefor
and?
that undergo desugaring expansion.)The current unexpansion code mostly works fine in that “flat” single-file single-context world. But it's not suitable for incremental work towards proper expansion-aware coverage instrumentation, which would allow a function's coverage spans to encompass multiple expansion contexts and multiple files.
This PR therefore replaces the current unexpansion code with a more sophisticated system that uses the raw MIR spans to reconstruct an “expansion tree”, and then uses that tree to help perform most of the unexpansion work.
Building the tree is “overkill” for current unexpansion needs (though it does give some minor edge-case improvements), but my hope is that having the explicit tree available will be a big help when taking the next steps towards proper expansion-region support.