Skip to content

lint direct use of rustc_type_ir #141614

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

Merged
merged 1 commit into from
Jun 19, 2025

Conversation

rperier
Copy link
Contributor

@rperier rperier commented May 26, 2025

cc #138449

As previously discussed with @lcnr, it is a lint to prevent direct use of rustc_type_ir, except for some internal crates (like next_trait_solver or rustc_middle for example).

@rustbot
Copy link
Collaborator

rustbot commented May 26, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels May 26, 2025
@compiler-errors
Copy link
Member

@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 May 26, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 26, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rperier
Copy link
Contributor Author

rperier commented May 28, 2025

@compiler-errors : Do I keep the existing lints for usage_of_type_ir_inherent and usage_of_type_ir_traits ? Or do I remove these, as they will implicitly be part of the lint "direct_use_of_rustc_type_ir" ? I mean the purpose of this new lint is to prevent the use of all types/traits under rustc_type_ir in random crates, including rustc_type_ir::inherent and rustc_type_ir::Interner , so we would have "one lint to rule them all" :p .

Also, diagnostic items seem to be only usable on exported modules, not the crate directly. So I gonna to add a diagnostic item on each public module exported by rustc_type_ir.

@compiler-errors
Copy link
Member

Also, diagnostic items seem to be only usable on exported modules, not the crate directly. So I gonna to add a diagnostic item on each public module exported by rustc_type_ir.

Please don't do this. You should be able to put a #![diagnostic_item] attr on the crate. If that doesn't work, then we need to fix the diagnostic item implementation.

Do I keep the existing lints for usage_of_type_ir_inherent and usage_of_type_ir_traits ? Or do I remove these, as they will implicitly be part of the lint "direct_use_of_rustc_type_ir" ?

Yes, you should keep them. They can be implemented in the same lint pass, though.

@fmease
Copy link
Member

fmease commented May 28, 2025

So I gonna to add a diagnostic item on each public module exported by rustc_type_ir.

This won't be sufficient because the crate root of rustc_type_ir not only contains modules but also a myriad of non-module items (which come from re-exports).

If #![cfg_attr(feature = "nightly", rustc_diagnostic_item = "type_ir")] in the crate root / root module of crate rustc_type_ir truly doesn't work, I would recommend patching the rustc_diagnostic_item logic in compiler/rustc_passes/src/diagnostic_items.rs. It currently goes through all the hir_crate_items() and off the top of my head I don't remember if it includes the LOCAL_CRATE.

@rperier
Copy link
Contributor Author

rperier commented May 28, 2025

So I gonna to add a diagnostic item on each public module exported by rustc_type_ir.

This won't be sufficient because the crate root of rustc_type_ir not only contains modules but also a myriad of non-module items (which come from re-exports).

Indeed, good point. The change itself is not too complicated, that's just I am discovering all in internal compiler types and architecture in the same time :D (but it's fun !)
Thank you both for your feedback and your responsiveness.

@rperier rperier closed this May 28, 2025
@rperier
Copy link
Contributor Author

rperier commented May 28, 2025

Sorry, bad manipulation

@rperier rperier reopened this May 28, 2025
@rperier
Copy link
Contributor Author

rperier commented May 28, 2025

You were right, it seems that `compiler/rustc_passes/src/diagnostic_items.rs' must be adapted to include the crate owner_id. I am still running non-regressions testing, it works now :) (otherwise the lint is not emitted at all)

@rperier rperier force-pushed the lint_type-ir-to-type-middle branch from 9fdb411 to b408e48 Compare June 2, 2025 18:34
@rperier
Copy link
Contributor Author

rperier commented Jun 2, 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 Jun 2, 2025
@rperier rperier requested a review from compiler-errors June 3, 2025 06:12
@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 Jun 11, 2025
@bors
Copy link
Collaborator

bors commented Jun 17, 2025

☔ The latest upstream changes (presumably #137944) made this pull request unmergeable. Please resolve the merge conflicts.

@rperier rperier force-pushed the lint_type-ir-to-type-middle branch from b408e48 to 02239a1 Compare June 18, 2025 13:54
This commit adds a lint to prevent the use of rustc_type_ir in random
compiler crates, except for type system internals traits, which are
explicitly allowed. Moreover, this fixes diagnostic_items() to include
the CRATE_OWNER_ID, otherwise rustc_diagnostic_item attribute is ignored
on the crate root.
@rperier rperier force-pushed the lint_type-ir-to-type-middle branch from 02239a1 to a1a3bef Compare June 18, 2025 14:32
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 18, 2025

📌 Commit a1a3bef has been approved by compiler-errors

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2025
@fmease fmease linked an issue Jun 18, 2025 that may be closed by this pull request
bors added a commit that referenced this pull request Jun 18, 2025
Rollup of 6 pull requests

Successful merges:

 - #135656 (Add `-Z hint-mostly-unused` to tell rustc that most of a crate will go unused)
 - #138237 (Get rid of `EscapeDebugInner`.)
 - #141614 (lint direct use of rustc_type_ir )
 - #142123 (Implement initial support for timing sections (`--json=timings`))
 - #142377 (Try unremapping compiler sources)
 - #142674 (remove duplicate crash test)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7c46544 into rust-lang:master Jun 19, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lint/tidy check imports of rustc_type_ir and rustc_middle
6 participants