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.

Sign up for GitHub

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

Jump to bottom

Unbox and unwrap the contents of StatementKind::Coverage #122937

Merged
merged 1 commit into from
Mar 24, 2024
Merged

Unbox and unwrap the contents of StatementKind::Coverage #122937

merged 1 commit into from
Mar 24, 2024

Conversation

Zalathar
Copy link
Contributor

The payload of coverage statements was historically a structure with several fields, so it was boxed to avoid bloating StatementKind.

Now that the payload is a single relatively-small enum, we can replace Box<Coverage> with just CoverageKind.

This patch also adds a size assertion for StatementKind, to avoid accidentally bloating it in the future.

@rustbot label +A-code-coverage

@Zalathar
Unbox and unwrap the contents of StatementKind::Coverage
ab92699 
The payload of coverage statements was historically a structure with several
fields, so it was boxed to avoid bloating `StatementKind`.

Now that the payload is a single relatively-small enum, we can replace
`Box<Coverage>` with just `CoverageKind`.

This patch also adds a size assertion for `StatementKind`, to avoid
accidentally bloating it in the future.
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

r? @matthewjasper

rustbot has assigned @matthewjasper.
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 Mar 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Mar 23, 2024
@Zalathar
Copy link
Contributor Author

Changes to rustc_codegen_gcc are trivial API adjustments.

(Maybe I should give CoverageInfoBuilderMethods a default implementation someday.)

Zalathar
Zalathar commented Mar 23, 2024
View reviewed changes
compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
Comment on lines -190 to -192
// Ignore `ConstEvalCounter`s
| StatementKind::ConstEvalCounter
// Ignore `Nop`s
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that these comments were preventing rustfmt from formatting this match, presumably due to rust-lang/rustfmt#4119.

Since these particular comments are low-value anyway, I worked around the issue by just deleting them.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 24, 2024

r? @oli-obk

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 24, 2024

📌 Commit ab92699 has been approved by oli-obk

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 Mar 24, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 24, 2024
@matthiaskrgr
Rollup merge of rust-lang#122937 - Zalathar:unbox, r=oli-obk
78af630 
Unbox and unwrap the contents of `StatementKind::Coverage`

The payload of coverage statements was historically a structure with several fields, so it was boxed to avoid bloating `StatementKind`.

Now that the payload is a single relatively-small enum, we can replace `Box<Coverage>` with just `CoverageKind`.

This patch also adds a size assertion for `StatementKind`, to avoid accidentally bloating it in the future.

`@rustbot` label +A-code-coverage
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 24, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2024
@bors
Auto merge of rust-lang#123000 - matthiaskrgr:rollup-pa6cygu, r=matth…
83c1f7c 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#122737 (conditionally ignore fatal diagnostic in the SilentEmitter)
 - rust-lang#122757 (Fixed the `private-dependency` bug)
 - rust-lang#122886 (add test for rust-lang#90192)
 - rust-lang#122937 (Unbox and unwrap the contents of `StatementKind::Coverage`)
 - rust-lang#122949 (Add a regression test for rust-lang#117310)
 - rust-lang#122962 (Track run-make-support lib in common inputs stamp)
 - rust-lang#122977 (Rename `Arguments::as_const_str` to `as_statically_known_str`)
 - rust-lang#122983 (Fix build failure on ARM/AArch64/PowerPC/RISC-V FreeBSD/NetBSD)
 - rust-lang#122984 (panic-in-panic-hook: formatting a message that's just a string is risk-free)
 - rust-lang#122992 (std::thread: refine available_parallelism for solaris/illumos.)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 24, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2024
@bors
Auto merge of rust-lang#123004 - matthiaskrgr:rollup-s3v4p50, r=matth…
d36bdd1 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#122737 (conditionally ignore fatal diagnostic in the SilentEmitter)
 - rust-lang#122757 (Fixed the `private-dependency` bug)
 - rust-lang#122886 (add test for rust-lang#90192)
 - rust-lang#122937 (Unbox and unwrap the contents of `StatementKind::Coverage`)
 - rust-lang#122949 (Add a regression test for rust-lang#117310)
 - rust-lang#122962 (Track run-make-support lib in common inputs stamp)
 - rust-lang#122977 (Rename `Arguments::as_const_str` to `as_statically_known_str`)
 - rust-lang#122983 (Fix build failure on ARM/AArch64/PowerPC/RISC-V FreeBSD/NetBSD)
 - rust-lang#122984 (panic-in-panic-hook: formatting a message that's just a string is risk-free)
 - rust-lang#122992 (std::thread: refine available_parallelism for solaris/illumos.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 19d3827 into rust-lang:master Mar 24, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 24, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2024
@rust-timer
Unrolled build for rust-lang#122937
1029e8e 
Rollup merge of rust-lang#122937 - Zalathar:unbox, r=oli-obk

Unbox and unwrap the contents of `StatementKind::Coverage`

The payload of coverage statements was historically a structure with several fields, so it was boxed to avoid bloating `StatementKind`.

Now that the payload is a single relatively-small enum, we can replace `Box<Coverage>` with just `CoverageKind`.

This patch also adds a size assertion for `StatementKind`, to avoid accidentally bloating it in the future.

``@rustbot`` label +A-code-coverage
@Zalathar Zalathar deleted the unbox branch March 24, 2024 22:38
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 16, 2024
@Zalathar
Move size assertions for mir::syntax types into the same module
71df270 
A redundant size assertion for `StatementKind` was added in rust-lang#122937, because
the existing assertion was in a different module.

This patch cleans that up, and also moves the `TerminatorKind` assertion into
the same module where it belongs, to avoid the same thing happening again.
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 16, 2024
@Zalathar
Move size assertions for mir::syntax types into the same file
a0c7fc1 
A redundant size assertion for `StatementKind` was added in rust-lang#122937, because
the existing assertion was in a different file.

This patch cleans that up, and also moves the `TerminatorKind` assertion into
the same module where it belongs, to avoid the same thing happening again.
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 16, 2024
@Zalathar
Move size assertions for mir::syntax types into the same file
c0e7659 
A redundant size assertion for `StatementKind` was added in rust-lang#122937, because
the existing assertion was in a different file.

This patch cleans that up, and also moves the `TerminatorKind` assertion into
the same file where it belongs, to avoid the same thing happening again.
@Zalathar Zalathar mentioned this pull request Apr 16, 2024
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 16, 2024
@GuillaumeGomez
Rollup merge of rust-lang#124006 - Zalathar:static-assert, r=nnethercote
dd08347 
Move size assertions for `mir::syntax` types into the same file

A redundant size assertion for `StatementKind` was added in rust-lang#122937, because the existing assertion was in a different file.

This PR cleans that up, and also moves the `TerminatorKind` assertion into the same file where it belongs, to avoid the same thing happening again.

r? `@nnethercote`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2024
@rust-timer
Unrolled build for rust-lang#124006
90a83e0 
Rollup merge of rust-lang#124006 - Zalathar:static-assert, r=nnethercote

Move size assertions for `mir::syntax` types into the same file

A redundant size assertion for `StatementKind` was added in rust-lang#122937, because the existing assertion was in a different file.

This PR cleans that up, and also moves the `TerminatorKind` assertion into the same file where it belongs, to avoid the same thing happening again.

r? `@nnethercote`
GuillaumeGomez pushed a commit to GuillaumeGomez/rust that referenced this pull request Jul 10, 2024
@matthiaskrgr
Rollup merge of rust-lang#122937 - Zalathar:unbox, r=oli-obk
1ee4ae9 
Unbox and unwrap the contents of `StatementKind::Coverage`

The payload of coverage statements was historically a structure with several fields, so it was boxed to avoid bloating `StatementKind`.

Now that the payload is a single relatively-small enum, we can replace `Box<Coverage>` with just `CoverageKind`.

This patch also adds a size assertion for `StatementKind`, to avoid accidentally bloating it in the future.

``@rustbot`` label +A-code-coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Swatinem Swatinem Swatinem approved these changes

Assignees

@oli-obk oli-obk

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
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Zalathar @rustbot @oli-obk @bors @Swatinem @matthewjasper