Start using clippy on rustc

[Noratrieb]

Proposal

Thanks to the amazing work T-bootstrap, clippy now works properly in rust-lang/rust! This is great, we should start using it.

We should enforce clippy in CI with -Aclippy::all -Dclippy::correctness. This ensures that there is not much effort and has the highest value. This finds real bugs, like https://github.com/rust-lang/rust/pull/119447.

Running it locally doesn't show too many lints, mostly problems with clippy::unused_io_amount on slices (these should just use read_exact and some issues around bitflags (which should be resolved with the upgrade)). Fixing all of this is not too much effort and high value.

clippy::all has hundreds of warnings and it's not realistic to enforce this any time soon. It's also unclear whether we ever want that, and I would very much prefer not bikeshedding that right now, let's focus on clippy::correctness. Meanwhile, we're still happy to take fixes for clippy::all lints as long as they aren't too much of a burden to review.

cc @flip1995, this could cause some trouble on syncs if clippy ships new correctness lints that fire and we use in-tree clippy instead of beta clippy.

Mentors or Reviewers

anyone

Process

The main points of the Major Change Process are as follows:

• [x] File an issue describing the proposal.
• [ ] A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• [ ] Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

 @rustbot concern reason-for-concern 
 <description of the concern> 

Concerns can be lifted with:

 @rustbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

cc @rust-lang/compiler @rust-lang/compiler-contributors

[compiler-errors]

🙌

@rustbot second

i'm very in favor of allowing a limited amount of clippy lints to be enforced in the compiler

[matthiaskrgr]

I've been applying a lot of clippy fixes to the codebase in the past years and I would say the most interesting lint groups for rustc are perf, correctness, and complexity, in that order.

There are a lot of style or pedantic lints which may be "nice to have"™ but still would be a pain to be gating on.

Some general questions:

How do we want to control the set of lints that we apply on rustc? We could do this in-code with warn/allow attrs on entire crates to ignore a couple of FPs but also "turn on this particular lint for core for example. We could add global -W -A to flags to x.py clippy. Do we want [allow(clippy::xy)] sprinkled around rustcs codebase to dodge FPs?

How do we handle subtrees like rustfmt or rust-analyzer which are checked by x.py clippy but may have different code policies.

I don't think that a clippy update should be blocked by new TPs/FPs in clippy showing up in rustcs codebase, what will be the to-go way in such cases (allow the entire lint, throw some allow attrs into the relevant places..?)

[compiler-errors]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@matthiaskrgr: Do you mind copying all those questions over to the zulip thread so that we can discuss them in detail?

[apiraino]

@rustbot label -final-comment-period +major-change-accepted

[nnethercote]

This has been closed. What's the status?

• Is clippy is working reliably on rustc?
• Does it get invoked automatically as part of x.py test or something else?
• Does it run on CI?

[Noratrieb]

It has been closed because the proposal has been accepted. @Kobzol is working on implementing it.