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.

Sign up for GitHub

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

Jump to bottom

coverage: Improve handling of function/closure spans #120569

Merged
merged 5 commits into from
Feb 5, 2024
Merged

coverage: Improve handling of function/closure spans #120569

merged 5 commits into from
Feb 5, 2024

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Feb 2, 2024

This is a combination of some loosely-related changes that touch the same code:

  1. Make unexpansion of closure bodies more precise, by unexpanding back to the context of the closure declaration, instead of unexpanding all the way back to the top-level context. This preserves the way we handle async desugaring and closures containing a single bang-macro, while also giving better results for closures defined in macros.
  2. Skip the normal span-refinement code when dealing with the trivial outer part of an async function.
  3. Be more explicit about the fact that fn_sig_span has been extended to the start of the function body, and is not necessarily present.

@rustbot label +A-code-coverage

@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2024

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

@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 2, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Feb 2, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Feb 2, 2024

@bors r+ rollup

r? @oli-obk

@bors
Copy link
Contributor

bors commented Feb 2, 2024

📌 Commit 23fb894 has been approved by oli-obk

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 2, 2024

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 2, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 3, 2024
Merged
@Zalathar
Copy link
Contributor Author

Zalathar commented Feb 4, 2024

This was excluded from rollup #120620 because it has an incidental conflict with #120566 in some test headers.

Since that rollup will hopefully land relatively soon, I’ll just rebase this afterwards.

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2024
@bors
Auto merge of rust-lang#120620 - matthiaskrgr:rollup-sscmbsm, r=matth…
16c7256 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#119543 (add avx512fp16 to x86 target features)
 - rust-lang#120004 (Release notes for 1.76)
 - rust-lang#120562 (Revert unsound libcore changes)
 - rust-lang#120566 (coverage: Use normal `edition:` headers in coverage tests)
 - rust-lang#120570 (Suggest changing type to const parameters if we encounter a type in the trait bound position)
 - rust-lang#120571 (Miscellaneous diagnostics cleanups)
 - rust-lang#120573 (Remove `BorrowckErrors::tainted_by_errors`)
 - rust-lang#120592 (Remove unnecessary `.to_string()`/`.as_str()`s)
 - rust-lang#120610 (hir: Remove the generic type parameter from `MaybeOwned`)
 - rust-lang#120616 (Fix ICE on field access on a tainted type after const-eval failure)

Failed merges:

 - rust-lang#120569 (coverage: Improve handling of function/closure spans)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2024
@bors
Auto merge of rust-lang#120620 - matthiaskrgr:rollup-sscmbsm, r=matth…
4e3eed4 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#119543 (add avx512fp16 to x86 target features)
 - rust-lang#120004 (Release notes for 1.76)
 - rust-lang#120562 (Revert unsound libcore changes)
 - rust-lang#120566 (coverage: Use normal `edition:` headers in coverage tests)
 - rust-lang#120570 (Suggest changing type to const parameters if we encounter a type in the trait bound position)
 - rust-lang#120571 (Miscellaneous diagnostics cleanups)
 - rust-lang#120573 (Remove `BorrowckErrors::tainted_by_errors`)
 - rust-lang#120592 (Remove unnecessary `.to_string()`/`.as_str()`s)
 - rust-lang#120610 (hir: Remove the generic type parameter from `MaybeOwned`)
 - rust-lang#120616 (Fix ICE on field access on a tainted type after const-eval failure)

Failed merges:

 - rust-lang#120569 (coverage: Improve handling of function/closure spans)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 4, 2024

☔ The latest upstream changes (presumably #120620) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 4, 2024
@Zalathar
coverage: Test for closure body that is a single bang-macro
fe420dc
@Zalathar
coverage: Add a test for #[coverage(..)] on closures
8dd2b37
@Zalathar
coverage: Make unexpansion of closure bodies more precise
dd6d7f2 
This improves the coverage instrumentation of closures declared in macros, as
seen in `closure_macro.rs` and `closure_macro_async.rs`.
@Zalathar
coverage: Hoist special handling of async function spans
fde1702 
This sidesteps the normal span refinement code in cases where we know that we
are only dealing with the special signature span that represents having called
an async function.
@Zalathar
coverage: Make fn_sig_span optional, and note its quirks
a246b6b
@Zalathar
Copy link
Contributor Author

Zalathar commented Feb 4, 2024

Rebased to resolve trivial conflict in a test file; no functional changes.

@Zalathar
Copy link
Contributor Author

Zalathar commented Feb 4, 2024

@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 4, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Feb 4, 2024

📌 Commit a246b6b has been approved by oli-obk

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 4, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 5, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
@bors
Auto merge of rust-lang#120660 - matthiaskrgr:rollup-zuqljul, r=matth…
8c0b4f6 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119481 (Clarify ambiguity in select_nth_unstable docs)
 - rust-lang#119600 (Remove outdated references to librustc_middle)
 - rust-lang#120458 (Document `&CStr` to `CString` conversion)
 - rust-lang#120569 (coverage: Improve handling of function/closure spans)
 - rust-lang#120572 (Update libc to 0.2.153)
 - rust-lang#120587 (miri: normalize struct tail in ABI compat check)
 - rust-lang#120607 (fix rust-lang#120603 by adding a check in default_read_buf)
 - rust-lang#120636 (Subtree update of `rust-analyzer`)
 - rust-lang#120641 (rustdoc: trait.impl, type.impl: sort impls to make it not depend on serialization order)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7294c15 into rust-lang:master Feb 5, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
@rust-timer
Unrolled build for rust-lang#120569
b36cac0 
Rollup merge of rust-lang#120569 - Zalathar:fn-sig, r=oli-obk

coverage: Improve handling of function/closure spans

This is a combination of some loosely-related changes that touch the same code:

1. Make unexpansion of closure bodies more precise, by unexpanding back to the context of the closure declaration, instead of unexpanding all the way back to the top-level context. This preserves the way we handle async desugaring and closures containing a single bang-macro, while also giving better results for closures defined in macros.
2. Skip the normal span-refinement code when dealing with the trivial outer part of an async function.
3. Be more explicit about the fact that `fn_sig_span` has been extended to the start of the function body, and is not necessarily present.

---

`@rustbot` label +A-code-coverage
@Zalathar Zalathar deleted the fn-sig branch February 5, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

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
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Zalathar @rustbot @oli-obk @bors @TaKO8Ki