coverage: Hoist some complex code out of the main span refinement loop by Zalathar · Pull Request #119208 · rust-lang/rust

@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 22, 2023

Swatinem

@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

Dec 28, 2023

@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

Dec 28, 2023

WaffleLapkin

Swatinem

@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

Jan 2, 2024

@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

Jan 3, 2024

@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

Jan 3, 2024

@Zalathar

@Zalathar

@Zalathar

@Zalathar

This draws a clear distinction between the fields/methods that are needed by
initial span extraction and preprocessing, and those that are needed by the
main "refinement" loop.
The struct itself is already non-public, so having public fields doesn't
achieve anything.

@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

Jan 6, 2024

This was referenced

Jan 6, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jan 6, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119208 (coverage: Hoist some complex code out of the main span refinement loop)
 - rust-lang#119216 (Use diagnostic namespace in stdlib)
 - rust-lang#119414 (bootstrap: Move -Clto= setting from Rustc::run to rustc_cargo)
 - rust-lang#119420 (Handle ForeignItem as TAIT scope.)
 - rust-lang#119468 (rustdoc-search: tighter encoding for f index)
 - rust-lang#119628 (remove duplicate test)
 - rust-lang#119638 (fix cyle error when suggesting to use associated function instead of constructor)
 - rust-lang#119640 (library: Fix warnings in rtstartup)
 - rust-lang#119642 (library: Fix a symlink test failing on Windows)

r? `@ghost`
`@rustbot` modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Jan 6, 2024
Rollup merge of rust-lang#119208 - Zalathar:hoist, r=WaffleLapkin,Swatinem

coverage: Hoist some complex code out of the main span refinement loop

The span refinement loop in `spans.rs` takes the spans that have been extracted from MIR, and modifies them to produce more helpful output in coverage reports.

It is also one of the most complicated pieces of code in the coverage instrumentor. It has an abundance of moving pieces that make it difficult to understand, and most attempts to modify it end up accidentally changing its behaviour in unacceptable ways.

This PR nevertheless tries to make a dent in it by hoisting two pieces of special-case logic out of the main loop, and into separate preprocessing passes. Coverage tests show that the resulting mappings are *almost* identical, with all known differences being unimportant.

This should hopefully unlock further simplifications to the refinement loop, since it now has fewer edge cases to worry about.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Jun 12, 2024
coverage: Replace the old span refiner with a single function

As more and more of the span refiner's functionality has been pulled out into separate early passes, it has finally reached the point where we can remove the rest of the old `SpansRefiner` code, and replace it with a single modestly-sized function.

~~There should be no change to the resulting coverage mappings, as demonstrated by the lack of changes to test output.~~

There is *almost* no change to the resulting coverage mappings. There are some minor changes to `loop` that on inspection appear to be neutral in terms of accuracy, with the old behaviour being a slightly-horrifying implementation detail of the old code, so I think they're acceptable.

Previous work in this direction includes:
- rust-lang#125921
- rust-lang#121019
- rust-lang#119208

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Jun 12, 2024
Rollup merge of rust-lang#126294 - Zalathar:spans-refiner, r=oli-obk

coverage: Replace the old span refiner with a single function

As more and more of the span refiner's functionality has been pulled out into separate early passes, it has finally reached the point where we can remove the rest of the old `SpansRefiner` code, and replace it with a single modestly-sized function.

~~There should be no change to the resulting coverage mappings, as demonstrated by the lack of changes to test output.~~

There is *almost* no change to the resulting coverage mappings. There are some minor changes to `loop` that on inspection appear to be neutral in terms of accuracy, with the old behaviour being a slightly-horrifying implementation detail of the old code, so I think they're acceptable.

Previous work in this direction includes:
- rust-lang#125921
- rust-lang#121019
- rust-lang#119208