Skip to content
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: Get hole spans from nested items without fully visiting them #137251

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Feb 19, 2025

This is a small simplification to the code that collects the spans of nested items within a function, so that those spans can be treated as “holes” to be avoided by the current function's coverage mappings.

The old code was using nested_filter::All to ensure that the visitor would see nested items. But we don't need the actual items themselves; we just need their spans, which we can obtain via a custom implementation of visit_nested_item.

This avoids the more expansive queries required by nested_filter::All.

@Zalathar Zalathar added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Feb 19, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 19, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 19, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 19, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@Zalathar
Copy link
Contributor Author

This PR came out of the realization that I now have a somewhat better understanding of HIR visitors than I did when I wrote the original code.

@Zalathar
Copy link
Contributor Author

Ah, it turns out that we do need nested_filter::OnlyBodies, to catch some obscure cases not covered by current tests, e.g. [0; const { 7 }] (const block inside "anon const").

@Zalathar
Copy link
Contributor Author

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 19, 2025
It turns out that this visitor doesn't actually need `nested_filter::All` to
handle nested items; it just needs to override `visit_nested_item` and look up
the item's span.
@Zalathar
Copy link
Contributor Author

Sadly this means we still have to use nested_filter::OnlyBodies after all, but at least now there's a test case describing why.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 19, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jieyouxu
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 19, 2025

📌 Commit d38f688 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 19, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 19, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#127793 (Added project-specific Zed IDE settings)
 - rust-lang#134995 (Stabilize const_slice_flatten)
 - rust-lang#136301 (Improve instant docs)
 - rust-lang#136347 (Add a bullet point to `std::fs::copy`)
 - rust-lang#136794 (Stabilize file_lock)
 - rust-lang#137094 (x86_win64 ABI: do not use xmm0 with softfloat ABI)
 - rust-lang#137227 (docs(dev): Update the feature-gate instructions)
 - rust-lang#137232 (Don't mention `FromResidual` on bad `?`)
 - rust-lang#137251 (coverage: Get hole spans from nested items without fully visiting them)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3fc6dfd into rust-lang:master Feb 19, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 19, 2025
Rollup merge of rust-lang#137251 - Zalathar:holes-visitor, r=jieyouxu

coverage: Get hole spans from nested items without fully visiting them

This is a small simplification to the code that collects the spans of nested items within a function, so that those spans can be treated as “holes” to be avoided by the current function's coverage mappings.

The old code was using `nested_filter::All` to ensure that the visitor would see nested items. But we don't need the actual items themselves; we just need their spans, which we can obtain via a custom implementation of `visit_nested_item`.

This avoids the more expansive queries required by `nested_filter::All`.
@rustbot rustbot added this to the 1.87.0 milestone Feb 19, 2025
@Zalathar Zalathar deleted the holes-visitor branch February 20, 2025 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants