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: Several small improvements to graph code #126538

Merged
merged 4 commits into from
Jun 17, 2024
Merged

Conversation

Zalathar
Copy link
Contributor

This PR combines a few small improvements to coverage graph handling code:

  • Remove some low-value implementation tests that were getting in the way of other changes.
  • Clean up pub visibility.
  • Flatten some code using let-else.
  • Prefer .copied() over .cloned().

@rustbot label +A-code-coverage

These tests might have originally been useful as an implementation aid, but now
they don't provide enough value to justify the burden of updating them as the
underlying code changes.

The code they test is still exercised by the main end-to-end coverage tests.
Using `pub(super)` makes it harder to move code between modules, and doesn't
provide much privacy benefit over `pub(crate)`.
@rustbot
Copy link
Collaborator

rustbot commented Jun 16, 2024

r? @nnethercote

rustbot has assigned @nnethercote.
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 Jun 16, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 16, 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 Jun 16, 2024
@nnethercote
Copy link
Contributor

Looks good, thanks for the nice descriptions of the changes.

I feel like I've been randomly assigned a lot of review relating to coverage lately. Am I just lucky? Or are there just lots of coverage PRs occurring and I'm getting the same number as other reviewers?

@bors r+

@bors
Copy link
Contributor

bors commented Jun 17, 2024

📌 Commit e5b43c3 has been approved by nnethercote

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 Jun 17, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 17, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#125258 (Resolve elided lifetimes in assoc const to static if no other lifetimes are in scope)
 - rust-lang#126250 (docs(change): Don't mention a Cargo 2024 edition change for 1.79)
 - rust-lang#126288 (doc: Added commas where needed)
 - rust-lang#126346 (export std::os::fd module on HermitOS)
 - rust-lang#126468 (div_euclid, rem_euclid: clarify/extend documentation)
 - rust-lang#126531 (Add codegen test for `Request::provide_*`)
 - rust-lang#126535 (coverage: Arrange span extraction/refinement as a series of passes)
 - rust-lang#126538 (coverage: Several small improvements to graph code)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d92aa56 into rust-lang:master Jun 17, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 17, 2024
Rollup merge of rust-lang#126538 - Zalathar:graph, r=nnethercote

coverage: Several small improvements to graph code

This PR combines a few small improvements to coverage graph handling code:
- Remove some low-value implementation tests that were getting in the way of other changes.
- Clean up `pub` visibility.
- Flatten some code using let-else.
- Prefer `.copied()` over `.cloned()`.

`@rustbot` label +A-code-coverage
@rustbot rustbot added this to the 1.81.0 milestone Jun 17, 2024
@Zalathar Zalathar deleted the graph branch June 17, 2024 06:40
@Zalathar
Copy link
Contributor Author

Zalathar commented Jun 17, 2024

I think it's just that most of my compiler PRs happen to be coverage-related, so if I have a burst of activity and the bot randomly picks you an above-average number of times, it looks like a spooky coincidence.

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