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: Check for async fn explicitly, without needing a heuristic #119155

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

Zalathar
Copy link
Contributor

The old code used a heuristic to detect async functions and adjust their coverage spans to produce better output. But there's no need to resort to a heuristic when we can just look back at the original definition and check whether the current function is actually an async fn.

In addition to being generally nicer, this also gets rid of the one piece of code that specifically cares about CoverageSpan::is_closure representing an actual closure. All remaining code that inspects that field just uses it as an indication that the span is a hole that should be carved out of other spans, and then discarded.

That opens up the possibility of introducing other kinds of “hole” spans, e.g. for nested functions/types/macros, and having them all behave uniformly.


@rustbot label +A-code-coverage

We have coverage tests that use async functions, but none that use async
blocks.
This reduces the risk of mixing up `fn_source_span` and `body_span`, and makes
it easier to pass along additional fields as needed.
The old code used a heuristic to detect async functions and adjust their
coverage spans to produce better output. But there's no need to resort to a
heuristic when we can just check whether the current function is actually an
`async fn`.
@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2023

r? @compiler-errors

(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 Dec 20, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2023

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 Dec 20, 2023
@Zalathar
Copy link
Contributor Author

Seeing #119034 (comment) makes me think this is going to get rerolled as well, but I'm not familiar with the etiquette of doing so myself.

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 20, 2023

📌 Commit cf6dc7a has been approved by compiler-errors

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 Dec 20, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 20, 2023
…llaumeGomez

Rollup of 3 pull requests

Successful merges:

 - rust-lang#119115 (Update documentation for `--env` compilation flag)
 - rust-lang#119155 (coverage: Check for `async fn` explicitly, without needing a heuristic)
 - rust-lang#119159 (Update LLVM submodule)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5906d8f into rust-lang:master Dec 20, 2023
11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 20, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 20, 2023
Rollup merge of rust-lang#119155 - Zalathar:async-fn, r=compiler-errors

coverage: Check for `async fn` explicitly, without needing a heuristic

The old code used a heuristic to detect async functions and adjust their coverage spans to produce better output. But there's no need to resort to a heuristic when we can just look back at the original definition and check whether the current function is actually an `async fn`.

In addition to being generally nicer, this also gets rid of the one piece of code that specifically cares about `CoverageSpan::is_closure` representing an actual closure. All remaining code that inspects that field just uses it as an indication that the span is a hole that should be carved out of other spans, and then discarded.

That opens up the possibility of introducing other kinds of “hole” spans, e.g. for nested functions/types/macros, and having them all behave uniformly.

---

`@rustbot` label +A-code-coverage
@Zalathar Zalathar deleted the async-fn branch December 20, 2023 21:58
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