-
Notifications
You must be signed in to change notification settings - Fork 13k
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
coverage: Be more strict about what counts as a "visible macro" #118595
Conversation
r? @TaKO8Ki (rustbot has picked a reviewer for you, use r? to override) |
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
@rustbot label +A-code-coverage |
1337501
to
a1b0bf1
Compare
a1b0bf1
to
242bff3
Compare
Thanks. @bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (6316ac8): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 674.182s -> 675.976s (0.27%) |
This is a follow-up to the workaround in #117827, and I believe it now properly fixes #117788.
The old code treats a span as having a “visible macro” if it is part of a macro-expansion, and its parent callsite's context is the same as the body span's context. But if the body span is itself part of an expansion, the macro in question might not actually be visible from the body span. That results in the macro name's length being meaningless as a span offset.
We now only consider spans whose parent callsite is the same as the source callsite, i.e. the parent has no parent.
I've also included some related cleanup for the code added by #117827. That code was more complicated than normal, because I wanted it to be easy to backport to stable/beta.