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: Tidy up early parts of the instrumentor pass #118929

Merged
merged 9 commits into from Dec 15, 2023

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Dec 14, 2023

This is extracted from #118237, which needed to be manually rebased anyway.

Unlike that PR, this one only affects the coverage instrumentor, and doesn't attempt to move any code into the MIR builder. That can be left to a future version of #118305, which can still benefit from these improvements.

So this is now mostly a refactoring of some internal parts of the instrumentor.

@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2023

r? @WaffleLapkin

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

rustbot commented Dec 14, 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 14, 2023
@WaffleLapkin
Copy link
Member

r? @cjgillot

@rustbot rustbot assigned cjgillot and unassigned WaffleLapkin Dec 14, 2023
@Zalathar Zalathar force-pushed the look-hir branch 3 times, most recently from ea1bfe9 to 719c196 Compare December 14, 2023 06:44
Changes in this patch:
  - Extract local variable `def_id`
  - Check `is_fn_like` without retrieving HIR
  - Inline some locals that are used once and aren't needed for clarity
We can just as easily look it up again from the source map and body span when
needed.
If we want to know whether two byte positions are in the same file, we don't
need to clone and compare `Lrc<SourceFile>`; we can just get their indices and
compare those instead.
This will normally be true, but in cases where it's not true we're better off
not making any assumptions about the signature.
@cjgillot
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 15, 2023

📌 Commit 684b9ea has been approved by cjgillot

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

Rollup of 3 pull requests

Successful merges:

 - rust-lang#116888 (Add discussion that concurrent access to the environment is unsafe)
 - rust-lang#118888 (Uplift `TypeAndMut` and `ClosureKind` to `rustc_type_ir`)
 - rust-lang#118929 (coverage: Tidy up early parts of the instrumentor pass)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6659b5e into rust-lang:master Dec 15, 2023
11 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2023
Rollup merge of rust-lang#118929 - Zalathar:look-hir, r=cjgillot

coverage: Tidy up early parts of the instrumentor pass

This is extracted from rust-lang#118237, which needed to be manually rebased anyway.

Unlike that PR, this one only affects the coverage instrumentor, and doesn't attempt to move any code into the MIR builder. That can be left to a future version of rust-lang#118305, which can still benefit from these improvements.

So this is now mostly a refactoring of some internal parts of the instrumentor.
@rustbot rustbot added this to the 1.76.0 milestone Dec 15, 2023
@Zalathar Zalathar deleted the look-hir branch December 15, 2023 09:17
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.

None yet

5 participants