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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dpaoliello
Copy link
Contributor

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
Copy link
Contributor

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
Contributor

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

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
Contributor

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
Contributor

@bors r+

@bors
Copy link
Contributor

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
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
Copy link
Member

@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
Contributor

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

@dpaoliello
Copy link
Contributor Author

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants