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.

Sign up for GitHub

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

Enable various new clippy lints #1025

Open
wants to merge 7 commits into
base: rust-next
Choose a base branch
from
Open

Enable various new clippy lints #1025

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3754849
rust: move 'rust_common_flags' to a separate configuration file
tgross35 Jul 16, 2023
82157b8
rust: clippy: enable lints from 'pedantic' that have no errors
tgross35 Jul 16, 2023
bbc6d1c
rust: clippy: enable 'undocumented_unsafe_blocks' lint
tgross35 Jul 16, 2023
8f4700d
rust: clippy: enable three trivial lints with corrections
tgross35 Jul 16, 2023
a764f0a
rust: clippy: enable two pointer-related lints
tgross35 Jul 16, 2023
2118d15
rust: clippy: enable lints related to improving style
tgross35 Jul 16, 2023
b2d5dbc
rust: clippy: enable 'items_after_statements' and 'redundant_closure_…
tgross35 Jul 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,19 +454,7 @@ KBUILD_USERLDFLAGS := $(USERLDFLAGS)

# These flags apply to all Rust code in the tree, including the kernel and
# host programs.
export rust_common_flags := --edition=2021 \
-Zbinary_dep_depinfo=y \
-Dunsafe_op_in_unsafe_fn -Drust_2018_idioms \
-Dunreachable_pub -Dnon_ascii_idents \
-Wmissing_docs \
-Drustdoc::missing_crate_level_docs \
-Dclippy::correctness -Dclippy::style \
-Dclippy::suspicious -Dclippy::complexity \
-Dclippy::perf \
-Dclippy::let_unit_value -Dclippy::mut_mut \
-Dclippy::needless_bitwise_bool \
-Dclippy::needless_continue \
-Wclippy::dbg_macro
export rust_common_flags := $(shell grep -v '^#\|^$$' $(srctree)/rust/rust_common_flags)

KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS)
KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS)
Expand Down
32 changes: 32 additions & 0 deletions rust/rust_common_flags
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# SPDX-License-Identifier: GPL-2.0
# Parameters passed to rust tools (`rustc`, `clippy`, `rustdoc`) whenever they
# are run.

# Standard configuration
--edition=2021
-Zbinary_dep_depinfo=y

# Standard rust lints
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps "rustc lints"?

-Dnon_ascii_idents
-Drust_2018_idioms
-Dunreachable_pub
-Dunsafe_op_in_unsafe_fn

# Documentation lints
-Wmissing_docs
-Drustdoc::missing_crate_level_docs

# Clippy groups
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A different patch series should split these into individual lints, unless there is a way to prevent Clippy from breaking compilation due to newer lints.

Perhaps we should enable individual them with -D, and keep the groups with -W? That way the new ones would be warnings only (assuming the flags work that way).

-Dclippy::complexity
-Dclippy::correctness
-Dclippy::perf
-Dclippy::style
-Dclippy::suspicious

# Clippy lints from pedantic
-Dclippy::mut_mut
-Dclippy::needless_bitwise_bool
-Dclippy::needless_continue

# Clippy lints that we only warn on
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we would explain why we only warn on, but we can do that later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I didn't exactly know the reasoning :) what is it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For dbg_macro (and todo I guess), it is to avoid annoying people while developing. They could avoid CLIPPY=1, but the intention is that you can use it all the time (except when building production kernels, since Clippy does not guarantee to generate the same code).

Also, perhaps consider if others should be -W too for similar reasons. Or perhaps whether the reasoning is bad to being with! :)

-Wclippy::dbg_macro