Use workspace lints for crates in `compiler/` by nnethercote · Pull Request #138084 · rust-lang/rust

nnethercote · 2025-03-06T04:30:08Z

This is nicer and hopefully less error prone than specifying lints via bootstrap.

compiler/rustc_builtin_macros/Cargo.toml

petrochenkov · 2025-03-06T11:27:10Z

Maybe add a tidy check making sure that every compiler/*/Cargo.toml file has workspace = true?

jieyouxu

This approach seems reasonable to me. I opened #138106 to track an cleanup follow-up to handle them consistently, but is not a blocker for this PR.

EDIT: bootstrap thread: https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Consistent.20handling.20of.20compiler.2F*.20RUSTFLAGs.20and.20lints

compiler/rustc_builtin_macros/Cargo.toml

jieyouxu · 2025-03-06T15:58:10Z

Cargo.toml

+# NOTE: rustc-specific lints (e.g. `rustc::internal`) aren't supported by
+# Cargo. They are specified for `compiler/` crates in
+# `src/bootstrap/src/core/builder/cargo.rs`.


Question: do you know if we have a cargo issue to link to? Even if this isn't going to be implemented for a long time.

@ehuss said that #44690 is probably a blocker. I updated the comment to mention that issue.

jieyouxu · 2025-03-06T16:01:14Z

src/bootstrap/src/core/builder/cargo.rs

+            // NOTE: these flags are added to RUSTFLAGS, which is ignored when
+            // compiling proc macro crates such as `rustc_macros`,
+            // unfortunately.


Discussion: yeah this is unfortunate.

One way to do this is by unconditionally passing an arbitrary flag and asserting its presence in each compiler crate. Ugly, but it could work I think.

-- #138035 (comment)

We should probably investigate if we want to switch over to that approach to make sure the lints get consistently applied to all compiler crates.

But for this PR, it's not a blocker. I find the workspace lints approach for non-internal lints still less "magical" than doing all of them here.

jieyouxu · 2025-03-06T16:48:43Z

Marking as S-waiting-on-team for discussion in https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Consistent.20handling.20of.20compiler.2F*.20RUSTFLAGs.20and.20lints.

It was added in rust-lang#129523 to enable building on stable when there were `cfg(bootstrap)` occurrences in the crate. But those are gone now, so the section can be removed.

`llvm_enzyme` is now in the extra `check-cfg` list in bootstrap, so it doesn't need to be handled explicitly here.

By naming them in `[workspace.lints.rust]` in the top-level `Cargo.toml`, and then making all `compiler/` crates inherit them with `[lints] workspace = true`. (I omitted `rustc_codegen_{cranelift,gcc}`, because they're a bit different.) The advantages of this over the current approach: - It uses a standard Cargo feature, rather than special handling in bootstrap. So, easier to understand, and less likely to get accidentally broken in the future. - It works for proc macro crates. It's a shame it doesn't work for rustc-specific lints, as the comments explain.

And fix the new errors in the handful of crates that didn't have a `#![warn(unreachable_pub)]`.

(Except for `rustc_codegen_cranelift`.) It's no longer necessary now that `unreachable_pub` is in the workspace lints.

rustbot · 2025-03-07T21:46:36Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_sanitizers

cc @rust-lang/project-exploit-mitigations, @rcvalle

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in exhaustiveness checking

cc @Nadrieril

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @TaKO8Ki

Some changes occurred in compiler/rustc_attr_data_structures

cc @jdonszelmann

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

nnethercote · 2025-03-07T21:49:23Z

@jieyouxu: I think this is ready to go.

The llvm_enzyme issue is fixed. There's a new commit removing it from compiler/rustc_builtin_macros/Cargo.toml.
The Zulip discussion hasn't surfaced any alternative suggestions.
I agree with @petrochenkov that a tidy check would be good. It will be similar to the existing edition.rs check, and I'm happy to write it, but it won't happen until next week. If you're willing to accept that in a follow-up PR then we could get this PR in the queue now.

jieyouxu · 2025-03-08T01:11:03Z

Yes, I think the tidy check isn't a blocker and can be done separately. I'll file an issue so we don't forgor.

@rustbot label: -S-waiting-on-team +S-waiting-on-review

jieyouxu · 2025-03-08T12:33:09Z

@bors r+

bors · 2025-03-08T12:33:12Z

📌 Commit 8a3e033 has been approved by jieyouxu

It is now in the queue for this repository.

…youxu Use workspace lints for crates in `compiler/` This is nicer and hopefully less error prone than specifying lints via bootstrap. r? `@jieyouxu`

Rollup of 16 pull requests Successful merges: - rust-lang#122790 (Apply dllimport in ThinLTO) - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138233 (Windows: Don't link std (and run-make) against advapi32, except on win7) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138084 - nnethercote:workspace-lints, r=jieyouxu Use workspace lints for crates in `compiler/` This is nicer and hopefully less error prone than specifying lints via bootstrap. r? ``@jieyouxu``

Revert <rust-lang#138084> to buy time to consider options that avoids breaking downstream usages of cargo on distributed `rustc-src` artifacts, where such cargo invocations fail due to inability to inherit `lints` from workspace root manifest's `workspace.lints` (this is only valid for the source rust-lang/rust workspace, but not really the distributed `rustc-src` artifacts). This breakage was reported in <rust-lang#138304>. This reverts commit 48caf81, reversing changes made to c666287.

…=Noratrieb Revert "Use workspace lints for crates in `compiler/` rust-lang#138084" Revert <rust-lang#138084> to buy time to consider options that avoids breaking downstream usages of cargo on distributed `rustc-src` artifacts, where such cargo invocations fail due to inability to inherit `lints` from workspace root manifest's `workspace.lints` (this is only valid for the source rust-lang/rust workspace, but not really the distributed `rustc-src` artifacts). The problem is that the `rustc-src` component doesn't include the root `Cargo.toml` manifest. This breakage was reported in rust-lang#138304. This reverts commit 48caf81, reversing changes made to c666287. cc `@RalfJung` r? `@nnethercote` (sorry, I didn't consider this being a thing 💀)

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#137931 (Add remark for missing `llvm-tools` component re. `rustc_private` linker failures related to not finding LLVM libraries) - rust-lang#138138 (Pass `InferCtxt` to `InlineAsmCtxt` to properly taint on error) - rust-lang#138223 (Fix post-merge workflow) - rust-lang#138268 (Handle empty test suites in GitHub job summary report) - rust-lang#138278 (Delegation: fix ICE with invalid `MethodCall` generation) - rust-lang#138281 (Fix O(tests) stack usage in edition 2024 mergeable doctests) - rust-lang#138305 (Subtree update of `rust-analyzer`) - rust-lang#138306 (Revert "Use workspace lints for crates in `compiler/` rust-lang#138084") r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138306 - jieyouxu:revert-workspace-lints, r=Noratrieb Revert "Use workspace lints for crates in `compiler/` rust-lang#138084" Revert <rust-lang#138084> to buy time to consider options that avoids breaking downstream usages of cargo on distributed `rustc-src` artifacts, where such cargo invocations fail due to inability to inherit `lints` from workspace root manifest's `workspace.lints` (this is only valid for the source rust-lang/rust workspace, but not really the distributed `rustc-src` artifacts). The problem is that the `rustc-src` component doesn't include the root `Cargo.toml` manifest. This breakage was reported in rust-lang#138304. This reverts commit 48caf81, reversing changes made to c666287. cc `@RalfJung` r? `@nnethercote` (sorry, I didn't consider this being a thing 💀)

nnethercote · 2025-03-11T22:09:40Z

This ended up being reverted in #138306 for breaking miri and other external tools, and being superseded by #138331.

…ore, r=onur-ozkan,jieyouxu Use `RUSTC_LINT_FLAGS` more An alternative to the failed rust-lang#138084. Fixes rust-lang#138106. r? `@jieyouxu`

…ore, r=onur-ozkan,jieyouxu Use `RUSTC_LINT_FLAGS` more An alternative to the failed rust-lang#138084. Fixes rust-lang#138106. r? ``@jieyouxu``

…ore, r=onur-ozkan,jieyouxu Use `RUSTC_LINT_FLAGS` more An alternative to the failed rust-lang#138084. Fixes rust-lang#138106. r? ```@jieyouxu```

…ore, r=onur-ozkan,jieyouxu Use `RUSTC_LINT_FLAGS` more An alternative to the failed rust-lang#138084. Fixes rust-lang#138106. r? ````@jieyouxu````

…ore, r=onur-ozkan,jieyouxu Use `RUSTC_LINT_FLAGS` more An alternative to the failed rust-lang#138084. Fixes rust-lang#138106. r? `````@jieyouxu`````

Rollup merge of rust-lang#138331 - nnethercote:use-RUSTC_LINT_FLAGS-more, r=onur-ozkan,jieyouxu Use `RUSTC_LINT_FLAGS` more An alternative to the failed rust-lang#138084. Fixes rust-lang#138106. r? ````@jieyouxu````

Revert "Use workspace lints for crates in `compiler/` #138084" Revert <rust-lang/rust#138084> to buy time to consider options that avoids breaking downstream usages of cargo on distributed `rustc-src` artifacts, where such cargo invocations fail due to inability to inherit `lints` from workspace root manifest's `workspace.lints` (this is only valid for the source rust-lang/rust workspace, but not really the distributed `rustc-src` artifacts). The problem is that the `rustc-src` component doesn't include the root `Cargo.toml` manifest. This breakage was reported in rust-lang/rust#138304. This reverts commit 48caf81, reversing changes made to c666287. cc `@RalfJung` r? `@nnethercote` (sorry, I didn't consider this being a thing 💀)

nnethercote mentioned this pull request Mar 6, 2025

Overhaul RUSTFLAGS handling for compiler/ crates #138035

Closed

rustbot assigned jieyouxu Mar 6, 2025

nnethercote commented Mar 6, 2025

View reviewed changes

compiler/rustc_builtin_macros/Cargo.toml Outdated Show resolved Hide resolved

jieyouxu approved these changes Mar 6, 2025

View reviewed changes

jieyouxu added S-waiting-on-team and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2025

nnethercote force-pushed the workspace-lints branch from 2b691ff to 3d2113f Compare March 7, 2025 01:55

nnethercote mentioned this pull request Mar 7, 2025

Use warn(unused_crate_dependencies) in compiler crates. #137911

Closed

nnethercote added 5 commits March 8, 2025 08:38

Remove [lints.rust] section from rustc_type_ir.

8af0aa2

It was added in rust-lang#129523 to enable building on stable when there were `cfg(bootstrap)` occurrences in the crate. But those are gone now, so the section can be removed.

Remove [lints.rust] section from rustc_builtin_macros.

a8eeb4b

`llvm_enzyme` is now in the extra `check-cfg` list in bootstrap, so it doesn't need to be handled explicitly here.

Add unreachable_pub to the default lints for compiler/ crates.

9212e31

And fix the new errors in the handful of crates that didn't have a `#![warn(unreachable_pub)]`.

Remove #![warn(unreachable_pub)] from all compiler/ crates.

8a3e033

(Except for `rustc_codegen_cranelift`.) It's no longer necessary now that `unreachable_pub` is in the workspace lints.

nnethercote force-pushed the workspace-lints branch from 3d2113f to 8a3e033 Compare March 7, 2025 21:45

nnethercote marked this pull request as ready for review March 7, 2025 21:46

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-team labels Mar 8, 2025

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 8, 2025

jieyouxu mentioned this pull request Mar 9, 2025

Rollup of 16 pull requests #138255

Closed

matthiaskrgr mentioned this pull request Mar 9, 2025

Rollup of 12 pull requests #138267

Merged

bors merged commit 48caf81 into rust-lang:master Mar 9, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 9, 2025

nnethercote deleted the workspace-lints branch March 9, 2025 21:08

RalfJung mentioned this pull request Mar 10, 2025

Manifests in rustc-src distributed package are not valid any more #138304

Closed

jieyouxu mentioned this pull request Mar 10, 2025

Revert "Use workspace lints for crates in compiler/ #138084" #138306

Merged

matthiaskrgr mentioned this pull request Mar 10, 2025

Rollup of 8 pull requests #138310

Merged

This was referenced Mar 11, 2025

Remove unnecessary [lints.rust] sections. #138330

Merged

Use RUSTC_LINT_FLAGS more #138331

Merged

Uh oh!

Conversation

nnethercote commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

petrochenkov commented Mar 6, 2025

Uh oh!

jieyouxu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jieyouxu Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

nnethercote Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Mar 6, 2025

Uh oh!

rustbot commented Mar 7, 2025

Uh oh!

nnethercote commented Mar 7, 2025

Uh oh!

jieyouxu commented Mar 8, 2025

Uh oh!

jieyouxu commented Mar 8, 2025

Uh oh!

bors commented Mar 8, 2025

Uh oh!

Uh oh!

nnethercote commented Mar 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nnethercote commented Mar 6, 2025 •

edited

Loading

jieyouxu left a comment •

edited

Loading