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: Don't use actual coverage spans in test_make_bcb_counters #115912

Closed
wants to merge 1 commit into from

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Sep 17, 2023

This test calls internal functions in spans in order to create a list of coverage spans to pass in when making BCB counters. That makes it difficult to modify those internals without breaking the test.

However, making BCB counters doesn't require the actual coverage spans; it just needs some way to identify which BCBs are associated with one or more spans. This can be achieved by passing in a Fn(BasicCoverageBlock) -> bool instead.

@rustbot
Copy link
Collaborator

rustbot commented Sep 17, 2023

r? @oli-obk

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

rustbot commented Sep 17, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@Zalathar
Copy link
Contributor Author

@rustbot label +A-code-coverage

@rustbot rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Sep 17, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This test calls internal functions in `spans` in order to create a list of
coverage spans to pass in when making BCB counters. That makes it difficult to
modify those internals without breaking the test.

However, making BCB counters doesn't require the actual coverage spans; it just
needs some way to identify which BCBs are associated with one or more spans.
This can be achieved by passing in a `Fn(BasicCoverageBlock) -> bool` instead.
@Zalathar
Copy link
Contributor Author

After more consideration, I'm going to fold this into #115301, because it ultimately ties into some spans changes that are linked to having multiple spans/regions per BCB.

@Zalathar Zalathar closed this Sep 18, 2023
@Zalathar Zalathar deleted the fake-spans branch September 18, 2023 10:05
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 19, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 20, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 21, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 21, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 21, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 23, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 23, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 24, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 25, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 26, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 27, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 27, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 28, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 29, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 30, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 1, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 1, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 1, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 2, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 3, 2023
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.

This patch includes some changes that were originally in rust-lang#115912, which avoid
the need for a particular test to deal with coverage spans at all.

(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
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-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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants