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
1 change: 1 addition & 0 deletions rust/kernel/init/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ macro_rules! __pin_data {
>,
}

#[allow(clippy::expl_impl_clone_on_copy)]
impl<$($impl_generics)*> ::core::clone::Clone for __ThePinData<$($ty_generics)*>
where $($whr)*
{
Expand Down
1 change: 1 addition & 0 deletions rust/kernel/print.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub mod format_strings {
/// given `prefix`, which are the kernel's `KERN_*` constants.
///
/// [`_printk`]: ../../../../include/linux/printk.h
#[allow(clippy::trivially_copy_pass_by_ref)]
const fn generate(is_cont: bool, prefix: &[u8; 3]) -> [u8; LENGTH] {
// Ensure the `KERN_*` macros are what we expect.
assert!(prefix[0] == b'\x01');
Expand Down
12 changes: 6 additions & 6 deletions rust/kernel/sync/arc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ impl<T: ?Sized> Deref for Arc<T> {

impl<T: ?Sized> AsRef<T> for Arc<T> {
fn as_ref(&self) -> &T {
self.deref()
self
}
}

Expand Down Expand Up @@ -595,7 +595,7 @@ impl<T: ?Sized> Deref for UniqueArc<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
self.inner.deref()
&self.inner
}
}

Expand All @@ -610,24 +610,24 @@ impl<T: ?Sized> DerefMut for UniqueArc<T> {

impl<T: fmt::Display + ?Sized> fmt::Display for UniqueArc<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self.deref(), f)
fmt::Display::fmt(&self, f)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm.. I don't think this is an improve, &self is of type &&UniqueArc<T>, it's not very straight-forwards why does the compiler coerce it to a &T. We could do the same as std:

fmt::Display::fmt(&**self, f);

But honestly, I don't think it's better.. could you explain what problem the old code has?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no particular problem other than keeping consistency, this comes from the explicit_deref_methods lint. I suspect that this is why std does what it does (I neglected to check what they do before applying this).

I am indifferent about this lint and agree that there are many times where it makes more sense to be explicit, so I don't mind dropping it. However, even unrelated to the lint, maybe it would be good to match what std does, just to avoid any possible confusion when syncing std -> kernel

}
}

impl<T: fmt::Display + ?Sized> fmt::Display for Arc<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self.deref(), f)
fmt::Display::fmt(&self, f)
}
}

impl<T: fmt::Debug + ?Sized> fmt::Debug for UniqueArc<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Debug::fmt(self.deref(), f)
fmt::Debug::fmt(&self, f)
}
}

impl<T: fmt::Debug + ?Sized> fmt::Debug for Arc<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Debug::fmt(self.deref(), f)
fmt::Debug::fmt(&self, f)
}
}
3 changes: 3 additions & 0 deletions rust/rust_common_flags
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
-Dclippy::doc_link_with_quotes
-Dclippy::doc_markdown
-Dclippy::empty_enum
-Dclippy::expl_impl_clone_on_copy
-Dclippy::explicit_deref_methods
-Dclippy::explicit_into_iter_loop
-Dclippy::explicit_iter_loop
-Dclippy::filter_map_next
Expand Down Expand Up @@ -67,6 +69,7 @@
-Dclippy::same_functions_in_if_condition
-Dclippy::stable_sort_primitive
-Dclippy::too_many_lines
-Dclippy::trivially_copy_pass_by_ref
-Dclippy::undocumented_unsafe_blocks
-Dclippy::unicode_not_nfc
-Dclippy::unnecessary_join
Expand Down