Skip to content

cg_llvm: Reduce the visibility of types, modules and using declarations in rustc_codegen_llvm.#137247

Merged
bors merged 1 commit intorust-lang:masterfrom
dpaoliello:cleanllvm
Mar 25, 2025
Merged

cg_llvm: Reduce the visibility of types, modules and using declarations in rustc_codegen_llvm.#137247
bors merged 1 commit intorust-lang:masterfrom
dpaoliello:cleanllvm

Conversation

@dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Feb 18, 2025 •
edited by Zalathar
Loading

Final part of #135502

Reduces the visibility of types, modules and using declarations in the rustc_codegen_llvm to private or pub(crate) where possible, and marks unused fields and enum entries with #[expect(dead_code)].

r? Zalathar

@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 Feb 18, 2025
Zalathar
Zalathar reviewed Feb 21, 2025
View reviewed changes
@Zalathar
Copy link
Member

Zalathar commented Feb 21, 2025

I've been thinking about how to deal with the enums that have more than 1-2 unused variants, especially the ones where almost all variants are unused.

Putting #[expect(dead_code)] on each individual variant is the most precise approach, but it has the massive practical downside of making the enums a nightmare to read/audit, which kind of defeats the point of wanting to keep them around while reducing visibility.

So for those enums, I think it's going to be better to just put #[expect(dead_code)] on the enum itself (instead of individual variants), and live with the fact that this makes it harder to see whether the enum is unused (or which variants are unused).

That's a bit of a shame, but it's still an improvement over the status quo of everything being pub.

@Zalathar
Copy link
Member

Zalathar commented Feb 21, 2025

Specifically, I'm proposing that these enums should have #[expect(dead_code)] on the enum itself, and not on its variants:

  • AttributeKind
  • TypeKind
  • MetadataType
  • Opcode

@dpaoliello
Copy link
Contributor Author

dpaoliello commented Feb 21, 2025

Specifically, I'm proposing that these enums should have #[expect(dead_code)] on the enum itself, and not on its variants:

  • AttributeKind
  • TypeKind
  • MetadataType
  • Opcode

Done

@Zalathar
Copy link
Member

Zalathar commented Feb 22, 2025

Adding reasons for the expect attributes was a great idea, but some of them were slightly inaccurate, so I pushed a fix for that (diff).

@Zalathar
Copy link
Member

Zalathar commented Feb 22, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 22, 2025

📌 Commit a5d4c5c has been approved by Zalathar

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 Feb 22, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 22, 2025
@matthiaskrgr
Rollup merge of rust-lang#137247 - dpaoliello:cleanllvm, r=Zalathar
f8061c5 
cg_llvm: Reduce the visibility of types, modules and using declarations in `rustc_codegen_llvm`.

Final part of rust-lang#135502

Reduces the visibility of types, modules and using declarations in the `rustc_codegen_llvm` to private or `pub(crate)` where possible, and marks unused fields and enum entries with `#[expect(dead_code)]`.

r? `@Zalathar`
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 22, 2025
Closed
@matthiaskrgr
Copy link
Member

matthiaskrgr commented Feb 22, 2025

@bors r-
#137431 (comment)

@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 Feb 22, 2025
@Zalathar
Copy link
Member

Zalathar commented Feb 23, 2025

Ah, this collided with some more pub items introduced by #136428.

@Zalathar Zalathar mentioned this pull request Feb 24, 2025
Closed
@dpaoliello
Copy link
Contributor Author

dpaoliello commented Feb 24, 2025

Ah, this collided with some more pub items introduced by #136428.

Updated.

@dpaoliello dpaoliello requested a review from Zalathar February 24, 2025 17:34
@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 Feb 24, 2025
@Zalathar
Copy link
Member

Zalathar commented Mar 21, 2025

Hmm, I think I lost track of this because more conflicts occurred shortly after the last rebase; it seems to need another rebase.

@Zalathar
Copy link
Member

Zalathar commented Mar 21, 2025

@rustbot author

@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 Mar 21, 2025
@dpaoliello
Copy link
Contributor Author

dpaoliello commented Mar 21, 2025

@rustbot ready

@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 Mar 21, 2025
@Zalathar
Copy link
Member

Zalathar commented Mar 22, 2025

Oh, could you please reapply the expect fixes from #137247 (comment) ? They probably got lost when you rebased from your local branch. (Squashing them into the main commit is fine.)

@dpaoliello
Copy link
Contributor Author

dpaoliello commented Mar 24, 2025

Oh, could you please reapply the expect fixes from #137247 (comment) ? They probably got lost when you rebased from your local branch. (Squashing them into the main commit is fine.)

Sorry about that: fixed.

@Zalathar
Copy link
Member

Zalathar commented Mar 25, 2025

Reapplied a clearer commit name, and fixed one expect reason (diff).

@Zalathar
Copy link
Member

Zalathar commented Mar 25, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 25, 2025

📌 Commit 79b9664 has been approved by Zalathar

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 25, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 25, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2025
@bors
Auto merge of rust-lang#138933 - matthiaskrgr:rollup-sjtqkoq, r=matth…
43f0014 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#135745 (Recognise new IPv6 non-global range from IETF RFC 9602)
 - rust-lang#137247 (cg_llvm: Reduce the visibility of types, modules and using declarations in `rustc_codegen_llvm`.)
 - rust-lang#138317 (privacy: Visit types and traits in impls in type privacy lints)
 - rust-lang#138581 (Abort in deadlock handler if we fail to get a query map)
 - rust-lang#138776 (coverage: Separate span-extraction from unexpansion)
 - rust-lang#138886 (Fix autofix for `self` and `self as …` in `unused_imports` lint)
 - rust-lang#138924 (Reduce `kw::Empty` usage, part 3)
 - rust-lang#138929 (Visitors track whether an assoc item is in a trait impl or an inherent impl)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b66e932 into rust-lang:master Mar 25, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 25, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2025
@rust-timer
Unrolled build for rust-lang#137247
2cf5148 
Rollup merge of rust-lang#137247 - dpaoliello:cleanllvm, r=Zalathar

cg_llvm: Reduce the visibility of types, modules and using declarations in `rustc_codegen_llvm`.

Final part of rust-lang#135502

Reduces the visibility of types, modules and using declarations in the `rustc_codegen_llvm` to private or `pub(crate)` where possible, and marks unused fields and enum entries with `#[expect(dead_code)]`.

r? Zalathar
@dpaoliello dpaoliello deleted the cleanllvm branch April 3, 2025 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Zalathar Zalathar Awaiting requested review from Zalathar

Assignees

@Zalathar Zalathar

Labels

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.87.0

Development

Successfully merging this pull request may close these issues.

5 participants

@dpaoliello @Zalathar @bors @matthiaskrgr @rustbot