[PATCH 00/19] rust: lint improvements

[PATCH 00/19] rust: lint improvements

[Miguel Ojeda]

Hi all,

This is a series that contains a series of lint-related things:

  - Cleanups and other improvements and fixes, including removing `allow`s that
    are not needed anymore for different reasons and a workaround for
    `dbg_macro` detection.

  - The enablement of some safety lints so that the toolchain enforces that we
    write `// SAFETY` comments and `# Safety` sections properly.

  - The addition of `.clippy.toml`, which allows us to take advantage of a few
    configuration options.

  - Start using the new `#[expect(...)]` feature and add documentation on it as
    well as lints in general.

Overall, this should improve the quality of the code and documentation as well
as reduce the time needed in reviews.

I want to mention Trevor's nice work on lints from a while ago [1]. I think we
should still do something like that: discuss which lints we would like to have
one-by-one and start enabling them (and perhaps have a file like Trevor proposed
etc.).

For the moment, though, I am sending these, since we would like to have at least
the safety-related ones enabled soon [2]: now that more code and developers
are joining, it sounds like a good time to start enforcing it -- it should make
new Rust kernel developers aware of the need of writing them, which has proven
to be a common request from reviewers.

If needed, the series can be applied partially or split, but most of it should
be fairly uncontroversial.

Link: https://github.com/Rust-for-Linux/linux/pull/1025 [1]
Link: https://lore.kernel.org/rust-for-linux/CD29DF8F-7FF3-466F-9724-BC92C14A68BD@collabora.com/ [2]

Miguel Ojeda (19):
  rust: workqueue: remove unneeded ``#[allow(clippy::new_ret_no_self)]`
  rust: sort global Rust flags
  rust: types: avoid repetition in `{As,From}Bytes` impls
  rust: enable `clippy::undocumented_unsafe_blocks` lint
  rust: enable `clippy::unnecessary_safety_comment` lint
  rust: enable `clippy::unnecessary_safety_doc` lint
  rust: enable `clippy::ignored_unit_patterns` lint
  rust: enable `rustdoc::unescaped_backticks` lint
  rust: init: remove unneeded `#[allow(clippy::disallowed_names)]`
  rust: sync: remove unneeded
    `#[allow(clippy::non_send_fields_in_send_ty)]`
  rust: introduce `.clippy.toml`
  rust: replace `clippy::dbg_macro` with `disallowed_macros`
  rust: rbtree: fix `SAFETY` comments that should be `# Safety` sections
  rust: provide proper code documentation titles
  rust: enable Clippy's `check-private-items`
  Documentation: rust: add coding guidelines on lints
  rust: start using the `#[expect(...)]` attribute
  Documentation: rust: discuss `#[expect(...)]` in the guidelines
  rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes

 .clippy.toml                             |   9 ++
 .gitignore                               |   1 +
 Documentation/rust/coding-guidelines.rst | 139 +++++++++++++++++++++++
 MAINTAINERS                              |   1 +
 Makefile                                 |  15 ++-
 rust/Makefile                            |   5 +-
 rust/bindings/lib.rs                     |   1 +
 rust/kernel/alloc/allocator.rs           |   2 +
 rust/kernel/error.rs                     |  11 +-
 rust/kernel/init.rs                      |  30 ++---
 rust/kernel/init/__internal.rs           |  11 +-
 rust/kernel/init/macros.rs               |  18 ++-
 rust/kernel/ioctl.rs                     |   2 +-
 rust/kernel/lib.rs                       |   1 +
 rust/kernel/list.rs                      |   1 +
 rust/kernel/list/arc_field.rs            |   2 +-
 rust/kernel/print.rs                     |   5 +-
 rust/kernel/rbtree.rs                    |   9 +-
 rust/kernel/std_vendor.rs                |  16 ++-
 rust/kernel/str.rs                       |   7 +-
 rust/kernel/sync/arc.rs                  |   2 +-
 rust/kernel/sync/arc/std_vendor.rs       |   2 +
 rust/kernel/sync/condvar.rs              |   1 -
 rust/kernel/sync/lock.rs                 |   6 +-
 rust/kernel/types.rs                     |  74 ++++++------
 rust/kernel/workqueue.rs                 |   9 +-
 rust/uapi/lib.rs                         |   1 +
 samples/rust/rust_print.rs               |   1 +
 scripts/Makefile.build                   |   2 +-
 29 files changed, 293 insertions(+), 91 deletions(-)
 create mode 100644 .clippy.toml


base-commit: 68d3b6aa08708bb3907c2c13eaf4b3ccf4805160
--
2.46.0

[PATCH 01/19] rust: workqueue: remove unneeded ``#[allow(clippy::new_ret_no_self)]`

[Miguel Ojeda]

Perform the same clean commit b2516f7af9d2 ("rust: kernel: remove
`#[allow(clippy::new_ret_no_self)]`") did for a case that appeared in
workqueue in parallel in commit 7324b88975c5 ("rust: workqueue: add
helper for defining work_struct fields"):

    Clippy triggered a false positive on its `new_ret_no_self` lint
    when using the `pin_init!` macro. Since Rust 1.67.0, that does
    not happen anymore, since Clippy learnt to not warn about
    `-> impl Trait<Self>` [1][2].

    The kernel nowadays uses Rust 1.72.1, thus remove the `#[allow]`.

    Link: https://github.com/rust-lang/rust-clippy/issues/7344 [1]
    Link: https://github.com/rust-lang/rust-clippy/pull/9733 [2]

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/workqueue.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 553a5cba2adc..493288dc1de0 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -366,7 +366,6 @@ unsafe impl<T: ?Sized, const ID: u64> Sync for Work<T, ID> {}
 impl<T: ?Sized, const ID: u64> Work<T, ID> {
     /// Creates a new instance of [`Work`].
     #[inline]
-    #[allow(clippy::new_ret_no_self)]
     pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self>
     where
         T: WorkItem<ID>,
-- 
2.46.0

Re: [PATCH 01/19] rust: workqueue: remove unneeded ``#[allow(clippy::new_ret_no_self)]`

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Perform the same clean commit b2516f7af9d2 ("rust: kernel: remove
> `#[allow(clippy::new_ret_no_self)]`") did for a case that appeared in
> workqueue in parallel in commit 7324b88975c5 ("rust: workqueue: add
> helper for defining work_struct fields"):
>
>     Clippy triggered a false positive on its `new_ret_no_self` lint
>     when using the `pin_init!` macro. Since Rust 1.67.0, that does
>     not happen anymore, since Clippy learnt to not warn about
>     `-> impl Trait<Self>` [1][2].
>
>     The kernel nowadays uses Rust 1.72.1, thus remove the `#[allow]`.
>
>     Link: https://github.com/rust-lang/rust-clippy/issues/7344 [1]
>     Link: https://github.com/rust-lang/rust-clippy/pull/9733 [2]
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 01/19] rust: workqueue: remove unneeded ``#[allow(clippy::new_ret_no_self)]`

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Perform the same clean commit b2516f7af9d2 ("rust: kernel: remove
> `#[allow(clippy::new_ret_no_self)]`") did for a case that appeared in
> workqueue in parallel in commit 7324b88975c5 ("rust: workqueue: add
> helper for defining work_struct fields"):
>
>     Clippy triggered a false positive on its `new_ret_no_self` lint
>     when using the `pin_init!` macro. Since Rust 1.67.0, that does
>     not happen anymore, since Clippy learnt to not warn about
>     `-> impl Trait<Self>` [1][2].
>
>     The kernel nowadays uses Rust 1.72.1, thus remove the `#[allow]`.
>
>     Link: https://github.com/rust-lang/rust-clippy/issues/7344 [1]
>     Link: https://github.com/rust-lang/rust-clippy/pull/9733 [2]
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 02/19] rust: sort global Rust flags

[Miguel Ojeda]

Sort the global Rust flags so that it is easier to follow along when we
have more, like this patch series does.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 68ebd6d6b444..7a97726f54f7 100644
--- a/Makefile
+++ b/Makefile
@@ -445,18 +445,18 @@ KBUILD_USERLDFLAGS := $(USERLDFLAGS)
 # host programs.
 export rust_common_flags := --edition=2021 \
 			    -Zbinary_dep_depinfo=y \
-			    -Dunsafe_op_in_unsafe_fn \
 			    -Dnon_ascii_idents \
+			    -Dunsafe_op_in_unsafe_fn \
+			    -Wmissing_docs \
 			    -Wrust_2018_idioms \
 			    -Wunreachable_pub \
-			    -Wmissing_docs \
-			    -Wrustdoc::missing_crate_level_docs \
 			    -Wclippy::all \
+			    -Wclippy::dbg_macro \
 			    -Wclippy::mut_mut \
 			    -Wclippy::needless_bitwise_bool \
 			    -Wclippy::needless_continue \
 			    -Wclippy::no_mangle_with_rust_abi \
-			    -Wclippy::dbg_macro
+			    -Wrustdoc::missing_crate_level_docs
 
 KBUILD_HOSTCFLAGS   := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
 		       $(HOSTCFLAGS) -I $(srctree)/scripts/include
-- 
2.46.0

Re: [PATCH 02/19] rust: sort global Rust flags

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Sort the global Rust flags so that it is easier to follow along when we
> have more, like this patch series does.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 02/19] rust: sort global Rust flags

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Sort the global Rust flags so that it is easier to follow along when we
> have more, like this patch series does.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 03/19] rust: types: avoid repetition in `{As,From}Bytes` impls

[Miguel Ojeda]

In order to provide `// SAFETY` comments for every `unsafe impl`, we would
need to repeat them, which is not very useful and would be harder to read.

We could perhaps allow the lint (ideally within a small module), but we
can take the chance to avoid the repetition of the `impl`s themselves
too by using a small local macro, like in other places where we have
had to do this sort of thing.

Thus add the straightforward `impl_{from,as}bytes!` macros and use them
to implement `FromBytes`.

This, in turn, will allow us in the next patch to place a `// SAFETY`
comment that defers to the actual invocation of the macro.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/types.rs | 68 +++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 9e7ca066355c..70e173f15d87 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -481,21 +481,22 @@ pub enum Either<L, R> {
 /// All bit-patterns must be valid for this type. This type must not have interior mutability.
 pub unsafe trait FromBytes {}
 
-// SAFETY: All bit patterns are acceptable values of the types below.
-unsafe impl FromBytes for u8 {}
-unsafe impl FromBytes for u16 {}
-unsafe impl FromBytes for u32 {}
-unsafe impl FromBytes for u64 {}
-unsafe impl FromBytes for usize {}
-unsafe impl FromBytes for i8 {}
-unsafe impl FromBytes for i16 {}
-unsafe impl FromBytes for i32 {}
-unsafe impl FromBytes for i64 {}
-unsafe impl FromBytes for isize {}
-// SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
-// patterns are also acceptable for arrays of that type.
-unsafe impl<T: FromBytes> FromBytes for [T] {}
-unsafe impl<T: FromBytes, const N: usize> FromBytes for [T; N] {}
+macro_rules! impl_frombytes {
+    ($($({$($generics:tt)*})? $t:ty, )*) => {
+        $(unsafe impl$($($generics)*)? FromBytes for $t {})*
+    };
+}
+
+impl_frombytes! {
+    // SAFETY: All bit patterns are acceptable values of the types below.
+    u8, u16, u32, u64, usize,
+    i8, i16, i32, i64, isize,
+
+    // SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
+    // patterns are also acceptable for arrays of that type.
+    {<T: FromBytes>} [T],
+    {<T: FromBytes, const N: usize>} [T; N],
+}
 
 /// Types that can be viewed as an immutable slice of initialized bytes.
 ///
@@ -514,21 +515,22 @@ unsafe impl<T: FromBytes> FromBytes for [T] {}
 /// mutability.
 pub unsafe trait AsBytes {}
 
-// SAFETY: Instances of the following types have no uninitialized portions.
-unsafe impl AsBytes for u8 {}
-unsafe impl AsBytes for u16 {}
-unsafe impl AsBytes for u32 {}
-unsafe impl AsBytes for u64 {}
-unsafe impl AsBytes for usize {}
-unsafe impl AsBytes for i8 {}
-unsafe impl AsBytes for i16 {}
-unsafe impl AsBytes for i32 {}
-unsafe impl AsBytes for i64 {}
-unsafe impl AsBytes for isize {}
-unsafe impl AsBytes for bool {}
-unsafe impl AsBytes for char {}
-unsafe impl AsBytes for str {}
-// SAFETY: If individual values in an array have no uninitialized portions, then the array itself
-// does not have any uninitialized portions either.
-unsafe impl<T: AsBytes> AsBytes for [T] {}
-unsafe impl<T: AsBytes, const N: usize> AsBytes for [T; N] {}
+macro_rules! impl_asbytes {
+    ($($({$($generics:tt)*})? $t:ty, )*) => {
+        $(unsafe impl$($($generics)*)? AsBytes for $t {})*
+    };
+}
+
+impl_asbytes! {
+    // SAFETY: Instances of the following types have no uninitialized portions.
+    u8, u16, u32, u64, usize,
+    i8, i16, i32, i64, isize,
+    bool,
+    char,
+    str,
+
+    // SAFETY: If individual values in an array have no uninitialized portions, then the array
+    // itself does not have any uninitialized portions either.
+    {<T: AsBytes>} [T],
+    {<T: AsBytes, const N: usize>} [T; N],
+}
-- 
2.46.0

Re: [PATCH 03/19] rust: types: avoid repetition in `{As,From}Bytes` impls

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In order to provide `// SAFETY` comments for every `unsafe impl`, we would
> need to repeat them, which is not very useful and would be harder to read.
>
> We could perhaps allow the lint (ideally within a small module), but we
> can take the chance to avoid the repetition of the `impl`s themselves
> too by using a small local macro, like in other places where we have
> had to do this sort of thing.
>
> Thus add the straightforward `impl_{from,as}bytes!` macros and use them
> to implement `FromBytes`.
>
> This, in turn, will allow us in the next patch to place a `// SAFETY`
> comment that defers to the actual invocation of the macro.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 03/19] rust: types: avoid repetition in `{As,From}Bytes` impls

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In order to provide `// SAFETY` comments for every `unsafe impl`, we would
> need to repeat them, which is not very useful and would be harder to read.
>
> We could perhaps allow the lint (ideally within a small module), but we
> can take the chance to avoid the repetition of the `impl`s themselves
> too by using a small local macro, like in other places where we have
> had to do this sort of thing.
>
> Thus add the straightforward `impl_{from,as}bytes!` macros and use them
> to implement `FromBytes`.
>
> This, in turn, will allow us in the next patch to place a `// SAFETY`
> comment that defers to the actual invocation of the macro.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 04/19] rust: enable `clippy::undocumented_unsafe_blocks` lint

[Miguel Ojeda]

Checking that we are not missing any `// SAFETY` comments in our `unsafe`
blocks is something we have wanted to do for a long time, as well as
cleaning up the remaining cases that were not documented [1].

Back when Rust for Linux started, this was something that could have
been done via a script, like Rust's `tidy`. Soon after, in Rust 1.58.0,
Clippy implemented the `undocumented_unsafe_blocks` lint [2].

Even though the lint has a few false positives, e.g. in some cases where
attributes appear between the comment and the `unsafe` block [3], there
are workarounds and the lint seems quite usable already.

Thus enable the lint now.

We still have a few cases to clean up, so just allow those for the moment
by writing a `TODO` comment -- some of those may be good candidates for
new contributors.

Link: https://github.com/Rust-for-Linux/linux/issues/351 [1]
Link: https://rust-lang.github.io/rust-clippy/master/#/undocumented_unsafe_blocks [2]
Link: https://github.com/rust-lang/rust-clippy/issues/13189 [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile                       | 1 +
 rust/bindings/lib.rs           | 1 +
 rust/kernel/alloc/allocator.rs | 2 ++
 rust/kernel/error.rs           | 9 ++++++---
 rust/kernel/init.rs            | 5 +++++
 rust/kernel/init/__internal.rs | 2 ++
 rust/kernel/init/macros.rs     | 9 +++++++++
 rust/kernel/list.rs            | 1 +
 rust/kernel/print.rs           | 2 ++
 rust/kernel/str.rs             | 7 ++++---
 rust/kernel/sync/condvar.rs    | 2 +-
 rust/kernel/sync/lock.rs       | 6 +++---
 rust/kernel/types.rs           | 4 ++++
 rust/kernel/workqueue.rs       | 4 ++++
 rust/uapi/lib.rs               | 1 +
 15 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 7a97726f54f7..c7a4f313728d 100644
--- a/Makefile
+++ b/Makefile
@@ -456,6 +456,7 @@ export rust_common_flags := --edition=2021 \
 			    -Wclippy::needless_bitwise_bool \
 			    -Wclippy::needless_continue \
 			    -Wclippy::no_mangle_with_rust_abi \
+			    -Wclippy::undocumented_unsafe_blocks \
 			    -Wrustdoc::missing_crate_level_docs
 
 KBUILD_HOSTCFLAGS   := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 93a1a3fc97bc..d6da3011281a 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -25,6 +25,7 @@
 )]
 
 #[allow(dead_code)]
+#[allow(clippy::undocumented_unsafe_blocks)]
 mod bindings_raw {
     // Use glob import here to expose all helpers.
     // Symbols defined within the module will take precedence to the glob import.
diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index e6ea601f38c6..91216b36af69 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -31,6 +31,7 @@ pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F
     unsafe { bindings::krealloc(ptr as *const core::ffi::c_void, size, flags.0) as *mut u8 }
 }
 
+// SAFETY: TODO.
 unsafe impl GlobalAlloc for KernelAllocator {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         // SAFETY: `ptr::null_mut()` is null and `layout` has a non-zero size by the function safety
@@ -39,6 +40,7 @@ unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
     }
 
     unsafe fn dealloc(&self, ptr: *mut u8, _layout: Layout) {
+        // SAFETY: TODO.
         unsafe {
             bindings::kfree(ptr as *const core::ffi::c_void);
         }
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 6f1587a2524e..639bc7572f90 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -171,9 +171,11 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.name() {
             // Print out number if no name can be found.
             None => f.debug_tuple("Error").field(&-self.0).finish(),
-            // SAFETY: These strings are ASCII-only.
             Some(name) => f
-                .debug_tuple(unsafe { core::str::from_utf8_unchecked(name) })
+                .debug_tuple(
+                    // SAFETY: These strings are ASCII-only.
+                    unsafe { core::str::from_utf8_unchecked(name) },
+                )
                 .finish(),
         }
     }
@@ -277,6 +279,8 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
     if unsafe { bindings::IS_ERR(const_ptr) } {
         // SAFETY: The FFI function does not deref the pointer.
         let err = unsafe { bindings::PTR_ERR(const_ptr) };
+
+        #[allow(clippy::unnecessary_cast)]
         // CAST: If `IS_ERR()` returns `true`,
         // then `PTR_ERR()` is guaranteed to return a
         // negative value greater-or-equal to `-bindings::MAX_ERRNO`,
@@ -286,7 +290,6 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
         //
         // SAFETY: `IS_ERR()` ensures `err` is a
         // negative value greater-or-equal to `-bindings::MAX_ERRNO`.
-        #[allow(clippy::unnecessary_cast)]
         return Err(unsafe { Error::from_errno_unchecked(err as core::ffi::c_int) });
     }
     Ok(ptr)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index a17ac8762d8f..08b9d695c285 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -541,6 +541,7 @@ macro_rules! stack_try_pin_init {
 /// }
 /// pin_init!(&this in Buf {
 ///     buf: [0; 64],
+///     // SAFETY: TODO.
 ///     ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
 ///     pin: PhantomPinned,
 /// });
@@ -875,6 +876,7 @@ pub unsafe trait PinInit<T: ?Sized, E = Infallible>: Sized {
     /// }
     ///
     /// let foo = pin_init!(Foo {
+    ///     // SAFETY: TODO.
     ///     raw <- unsafe {
     ///         Opaque::ffi_init(|s| {
     ///             init_foo(s);
@@ -1162,6 +1164,7 @@ pub fn pin_init_array_from_fn<I, const N: usize, T, E>(
 // SAFETY: Every type can be initialized by-value.
 unsafe impl<T, E> Init<T, E> for T {
     unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
+        // SAFETY: TODO.
         unsafe { slot.write(self) };
         Ok(())
     }
@@ -1170,6 +1173,7 @@ unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
 // SAFETY: Every type can be initialized by-value. `__pinned_init` calls `__init`.
 unsafe impl<T, E> PinInit<T, E> for T {
     unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
+        // SAFETY: TODO.
         unsafe { self.__init(slot) }
     }
 }
@@ -1411,6 +1415,7 @@ pub fn zeroed<T: Zeroable>() -> impl Init<T> {
 
 macro_rules! impl_zeroable {
     ($($({$($generics:tt)*})? $t:ty, )*) => {
+        // SAFETY: Safety comments written in the macro invocation.
         $(unsafe impl$($($generics)*)? Zeroable for $t {})*
     };
 }
diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
index 13cefd37512f..29f4fd00df3d 100644
--- a/rust/kernel/init/__internal.rs
+++ b/rust/kernel/init/__internal.rs
@@ -112,10 +112,12 @@ fn clone(&self) -> Self {
 
 impl<T: ?Sized> Copy for AllData<T> {}
 
+// SAFETY: TODO.
 unsafe impl<T: ?Sized> InitData for AllData<T> {
     type Datee = T;
 }
 
+// SAFETY: TODO.
 unsafe impl<T: ?Sized> HasInitData for T {
     type InitData = AllData<T>;
 
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 02ecedc4ae7a..93af8f3d8d4d 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -513,6 +513,7 @@ fn drop($($sig:tt)*) {
             }
         ),
     ) => {
+        // SAFETY: TODO.
         unsafe $($impl_sig)* {
             // Inherit all attributes and the type/ident tokens for the signature.
             $(#[$($attr)*])*
@@ -872,6 +873,7 @@ unsafe fn __pin_data() -> Self::PinData {
                 }
             }
 
+            // SAFETY: TODO.
             unsafe impl<$($impl_generics)*>
                 $crate::init::__internal::PinData for __ThePinData<$($ty_generics)*>
             where $($whr)*
@@ -997,6 +999,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
                     slot: *mut $p_type,
                     init: impl $crate::init::PinInit<$p_type, E>,
                 ) -> ::core::result::Result<(), E> {
+                    // SAFETY: TODO.
                     unsafe { $crate::init::PinInit::__pinned_init(init, slot) }
                 }
             )*
@@ -1007,6 +1010,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
                     slot: *mut $type,
                     init: impl $crate::init::Init<$type, E>,
                 ) -> ::core::result::Result<(), E> {
+                    // SAFETY: TODO.
                     unsafe { $crate::init::Init::__init(init, slot) }
                 }
             )*
@@ -1121,6 +1125,8 @@ macro_rules! __init_internal {
         // no possibility of returning without `unsafe`.
         struct __InitOk;
         // Get the data about fields from the supplied type.
+        //
+        // SAFETY: TODO.
         let data = unsafe {
             use $crate::init::__internal::$has_data;
             // Here we abuse `paste!` to retokenize `$t`. Declarative macros have some internal
@@ -1176,6 +1182,7 @@ fn assert_zeroable<T: $crate::init::Zeroable>(_: *mut T) {}
         let init = move |slot| -> ::core::result::Result<(), $err> {
             init(slot).map(|__InitOk| ())
         };
+        // SAFETY: TODO.
         let init = unsafe { $crate::init::$construct_closure::<_, $err>(init) };
         init
     }};
@@ -1324,6 +1331,8 @@ fn assert_zeroable<T: $crate::init::Zeroable>(_: *mut T) {}
         // Endpoint, nothing more to munch, create the initializer.
         // Since we are in the closure that is never called, this will never get executed.
         // We abuse `slot` to get the correct type inference here:
+        //
+        // SAFETY: TODO.
         unsafe {
             // Here we abuse `paste!` to retokenize `$t`. Declarative macros have some internal
             // information that is associated to already parsed fragments, so a path fragment
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index 5b4aec29eb67..fb93330f4af4 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -354,6 +354,7 @@ pub fn pop_front(&mut self) -> Option<ListArc<T, ID>> {
     ///
     /// `item` must not be in a different linked list (with the same id).
     pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
+        // SAFETY: TODO.
         let mut item = unsafe { ListLinks::fields(T::view_links(item)) };
         // SAFETY: The user provided a reference, and reference are never dangling.
         //
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 508b0221256c..fe53fc469c4f 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -23,6 +23,7 @@
     use fmt::Write;
     // SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
     let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
+    // SAFETY: TODO.
     let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
     w.pos().cast()
 }
@@ -102,6 +103,7 @@ pub unsafe fn call_printk(
 ) {
     // `_printk` does not seem to fail in any path.
     #[cfg(CONFIG_PRINTK)]
+    // SAFETY: TODO.
     unsafe {
         bindings::_printk(
             format_string.as_ptr() as _,
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index bb8d4f41475b..66d4527f6c6f 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -162,10 +162,10 @@ pub const fn len(&self) -> usize {
     /// Returns the length of this string with `NUL`.
     #[inline]
     pub const fn len_with_nul(&self) -> usize {
-        // SAFETY: This is one of the invariant of `CStr`.
-        // We add a `unreachable_unchecked` here to hint the optimizer that
-        // the value returned from this function is non-zero.
         if self.0.is_empty() {
+            // SAFETY: This is one of the invariant of `CStr`.
+            // We add a `unreachable_unchecked` here to hint the optimizer that
+            // the value returned from this function is non-zero.
             unsafe { core::hint::unreachable_unchecked() };
         }
         self.0.len()
@@ -301,6 +301,7 @@ pub fn to_str(&self) -> Result<&str, core::str::Utf8Error> {
     /// ```
     #[inline]
     pub unsafe fn as_str_unchecked(&self) -> &str {
+        // SAFETY: TODO.
         unsafe { core::str::from_utf8_unchecked(self.as_bytes()) }
     }
 
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index 2b306afbe56d..7e00048bf4b1 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -92,8 +92,8 @@ pub struct CondVar {
     _pin: PhantomPinned,
 }
 
-// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on any thread.
 #[allow(clippy::non_send_fields_in_send_ty)]
+// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on any thread.
 unsafe impl Send for CondVar {}
 
 // SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on multiple threads
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index f6c34ca4d819..07fcf2d8efc6 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -150,9 +150,9 @@ pub(crate) fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
         // SAFETY: The caller owns the lock, so it is safe to unlock it.
         unsafe { B::unlock(self.lock.state.get(), &self.state) };
 
-        // SAFETY: The lock was just unlocked above and is being relocked now.
-        let _relock =
-            ScopeGuard::new(|| unsafe { B::relock(self.lock.state.get(), &mut self.state) });
+        let _relock = ScopeGuard::new(||
+                // SAFETY: The lock was just unlocked above and is being relocked now.
+                unsafe { B::relock(self.lock.state.get(), &mut self.state) });
 
         cb()
     }
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 70e173f15d87..6c2d5fa9bce3 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -410,6 +410,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// struct Empty {}
     ///
+    /// # // SAFETY: TODO.
     /// unsafe impl AlwaysRefCounted for Empty {
     ///     fn inc_ref(&self) {}
     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
@@ -417,6 +418,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// let mut data = Empty {};
     /// let ptr = NonNull::<Empty>::new(&mut data as *mut _).unwrap();
+    /// # // SAFETY: TODO.
     /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
     /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
     ///
@@ -483,6 +485,7 @@ pub unsafe trait FromBytes {}
 
 macro_rules! impl_frombytes {
     ($($({$($generics:tt)*})? $t:ty, )*) => {
+        // SAFETY: Safety comments written in the macro invocation.
         $(unsafe impl$($($generics)*)? FromBytes for $t {})*
     };
 }
@@ -517,6 +520,7 @@ pub unsafe trait AsBytes {}
 
 macro_rules! impl_asbytes {
     ($($({$($generics:tt)*})? $t:ty, )*) => {
+        // SAFETY: Safety comments written in the macro invocation.
         $(unsafe impl$($($generics)*)? AsBytes for $t {})*
     };
 }
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 493288dc1de0..3b3f1dbe8192 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -519,6 +519,7 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ
     impl{T} HasWork<Self> for ClosureWork<T> { self.work }
 }
 
+// SAFETY: TODO.
 unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
 where
     T: WorkItem<ID, Pointer = Self>,
@@ -536,6 +537,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
     }
 }
 
+// SAFETY: TODO.
 unsafe impl<T, const ID: u64> RawWorkItem<ID> for Arc<T>
 where
     T: WorkItem<ID, Pointer = Self>,
@@ -564,6 +566,7 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput
     }
 }
 
+// SAFETY: TODO.
 unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<Box<T>>
 where
     T: WorkItem<ID, Pointer = Self>,
@@ -583,6 +586,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<Box<T>>
     }
 }
 
+// SAFETY: TODO.
 unsafe impl<T, const ID: u64> RawWorkItem<ID> for Pin<Box<T>>
 where
     T: WorkItem<ID, Pointer = Self>,
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
index 80a00260e3e7..fea2de330d19 100644
--- a/rust/uapi/lib.rs
+++ b/rust/uapi/lib.rs
@@ -14,6 +14,7 @@
 #![cfg_attr(test, allow(unsafe_op_in_unsafe_fn))]
 #![allow(
     clippy::all,
+    clippy::undocumented_unsafe_blocks,
     dead_code,
     missing_docs,
     non_camel_case_types,
-- 
2.46.0

Re: [PATCH 04/19] rust: enable `clippy::undocumented_unsafe_blocks` lint

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Checking that we are not missing any `// SAFETY` comments in our `unsafe`
> blocks is something we have wanted to do for a long time, as well as
> cleaning up the remaining cases that were not documented [1].
>
> Back when Rust for Linux started, this was something that could have
> been done via a script, like Rust's `tidy`. Soon after, in Rust 1.58.0,
> Clippy implemented the `undocumented_unsafe_blocks` lint [2].
>
> Even though the lint has a few false positives, e.g. in some cases where
> attributes appear between the comment and the `unsafe` block [3], there
> are workarounds and the lint seems quite usable already.
>
> Thus enable the lint now.
>
> We still have a few cases to clean up, so just allow those for the moment
> by writing a `TODO` comment -- some of those may be good candidates for
> new contributors.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/351 [1]
> Link: https://rust-lang.github.io/rust-clippy/master/#/undocumented_unsafe_blocks [2]
> Link: https://github.com/rust-lang/rust-clippy/issues/13189 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 04/19] rust: enable `clippy::undocumented_unsafe_blocks` lint

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Checking that we are not missing any `// SAFETY` comments in our `unsafe`
> blocks is something we have wanted to do for a long time, as well as
> cleaning up the remaining cases that were not documented [1].
>
> Back when Rust for Linux started, this was something that could have
> been done via a script, like Rust's `tidy`. Soon after, in Rust 1.58.0,
> Clippy implemented the `undocumented_unsafe_blocks` lint [2].
>
> Even though the lint has a few false positives, e.g. in some cases where
> attributes appear between the comment and the `unsafe` block [3], there
> are workarounds and the lint seems quite usable already.
>
> Thus enable the lint now.
>
> We still have a few cases to clean up, so just allow those for the moment
> by writing a `TODO` comment -- some of those may be good candidates for
> new contributors.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/351 [1]
> Link: https://rust-lang.github.io/rust-clippy/master/#/undocumented_unsafe_blocks [2]
> Link: https://github.com/rust-lang/rust-clippy/issues/13189 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

> @@ -277,6 +279,8 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
>      if unsafe { bindings::IS_ERR(const_ptr) } {
>          // SAFETY: The FFI function does not deref the pointer.
>          let err = unsafe { bindings::PTR_ERR(const_ptr) };
> +
> +        #[allow(clippy::unnecessary_cast)]
>          // CAST: If `IS_ERR()` returns `true`,
>          // then `PTR_ERR()` is guaranteed to return a
>          // negative value greater-or-equal to `-bindings::MAX_ERRNO`,
> @@ -286,7 +290,6 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
>          //
>          // SAFETY: `IS_ERR()` ensures `err` is a
>          // negative value greater-or-equal to `-bindings::MAX_ERRNO`.
> -        #[allow(clippy::unnecessary_cast)]
>          return Err(unsafe { Error::from_errno_unchecked(err as core::ffi::c_int) });
>      }

There are a couple places where the attributes move like this - did
this come from an older clippy version? clippy used to warn about this
but accepts it by default since [1]. (Still works of course, I just
think it looks nicer to have the attributes next to their statements).

Either way:

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[1]: https://github.com/rust-lang/rust-clippy/pull/11170

Re: [PATCH 04/19] rust: enable `clippy::undocumented_unsafe_blocks` lint

[Miguel Ojeda]

On Sun, Sep 29, 2024 at 6:33 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> There are a couple places where the attributes move like this - did
> this come from an older clippy version? clippy used to warn about this
> but accepts it by default since [1]. (Still works of course, I just
> think it looks nicer to have the attributes next to their statements).

I started this series about a long time ago, but I don't think old
Clippy is the reason -- I took a look again at this a couple months
ago and created this issue about it due to these lines of code,
including some examples and test cases:

    https://github.com/rust-lang/rust-clippy/issues/13189

For instance:

    // SAFETY: ...
    #[allow(clippy::unnecessary_cast)]
    return Err(unsafe { Error::from_errno_unchecked(err as core::ffi::c_int) });

This will not work, i.e. it does not see the `// SAFETY:`. However,
putting the `#[allow]` on top to let the comment be closer will work
(like in the patch). This will also work:

    return Err(
        // SAFETY: ...
        #[allow(clippy::unnecessary_cast)]
        unsafe { Error::from_errno_unchecked(err as core::ffi::c_int) }
    );

There has been no reply in the issue so far, but if you have any
comments/insights on whether those cases should or should not work,
that would be great.

Cheers,
Miguel

[PATCH 05/19] rust: enable `clippy::unnecessary_safety_comment` lint

[Miguel Ojeda]

In Rust 1.67.0, Clippy added the `unnecessary_safety_comment` lint [1],
which is the "inverse" of `undocumented_unsafe_blocks`: it finds places
where safe code has a `// SAFETY` comment attached.

The lint currently finds 3 places where we had such mistakes, thus it
seems already quite useful.

Thus clean those and enable it.

Link: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_safety_comment [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile                 | 1 +
 rust/kernel/sync/arc.rs  | 2 +-
 rust/kernel/workqueue.rs | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index c7a4f313728d..85f37ac9fef8 100644
--- a/Makefile
+++ b/Makefile
@@ -457,6 +457,7 @@ export rust_common_flags := --edition=2021 \
 			    -Wclippy::needless_continue \
 			    -Wclippy::no_mangle_with_rust_abi \
 			    -Wclippy::undocumented_unsafe_blocks \
+			    -Wclippy::unnecessary_safety_comment \
 			    -Wrustdoc::missing_crate_level_docs
 
 KBUILD_HOSTCFLAGS   := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 3021f30fd822..84c7cbfb096d 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -341,7 +341,7 @@ fn into_foreign(self) -> *const core::ffi::c_void {
     }
 
     unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> {
-        // SAFETY: By the safety requirement of this function, we know that `ptr` came from
+        // By the safety requirement of this function, we know that `ptr` came from
         // a previous call to `Arc::into_foreign`.
         let inner = NonNull::new(ptr as *mut ArcInner<T>).unwrap();
 
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 3b3f1dbe8192..10d2bc62e2cf 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -526,7 +526,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
     T: HasWork<T, ID>,
 {
     unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
-        // SAFETY: The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
+        // The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
         let ptr = ptr as *mut Work<T, ID>;
         // SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
         let ptr = unsafe { T::work_container_of(ptr) };
@@ -573,7 +573,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<Box<T>>
     T: HasWork<T, ID>,
 {
     unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
-        // SAFETY: The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
+        // The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
         let ptr = ptr as *mut Work<T, ID>;
         // SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
         let ptr = unsafe { T::work_container_of(ptr) };
-- 
2.46.0

Re: [PATCH 05/19] rust: enable `clippy::unnecessary_safety_comment` lint

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.67.0, Clippy added the `unnecessary_safety_comment` lint [1],
> which is the "inverse" of `undocumented_unsafe_blocks`: it finds places
> where safe code has a `// SAFETY` comment attached.
>
> The lint currently finds 3 places where we had such mistakes, thus it
> seems already quite useful.
>
> Thus clean those and enable it.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_safety_comment [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 05/19] rust: enable `clippy::unnecessary_safety_comment` lint

[Vincenzo Palazzo]

> In Rust 1.67.0, Clippy added the `unnecessary_safety_comment` lint [1],
> which is the "inverse" of `undocumented_unsafe_blocks`: it finds places
> where safe code has a `// SAFETY` comment attached.
>
> The lint currently finds 3 places where we had such mistakes, thus it
> seems already quite useful.
>
> Thus clean those and enable it.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_safety_comment [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

Re: [PATCH 05/19] rust: enable `clippy::unnecessary_safety_comment` lint

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.67.0, Clippy added the `unnecessary_safety_comment` lint [1],
> which is the "inverse" of `undocumented_unsafe_blocks`: it finds places
> where safe code has a `// SAFETY` comment attached.
>
> The lint currently finds 3 places where we had such mistakes, thus it
> seems already quite useful.
>
> Thus clean those and enable it.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_safety_comment [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 06/19] rust: enable `clippy::unnecessary_safety_doc` lint

[Miguel Ojeda]

In Rust 1.67.0, Clippy added the `unnecessary_safety_doc` lint [1],
which is similar to `unnecessary_safety_comment`, but for `# Safety`
sections, i.e. safety preconditions in the documentation.

This is something that should not happen with our coding guidelines in
mind. Thus enable the lint to have it machine-checked.

Link: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_safety_doc [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 85f37ac9fef8..f903309db91b 100644
--- a/Makefile
+++ b/Makefile
@@ -458,6 +458,7 @@ export rust_common_flags := --edition=2021 \
 			    -Wclippy::no_mangle_with_rust_abi \
 			    -Wclippy::undocumented_unsafe_blocks \
 			    -Wclippy::unnecessary_safety_comment \
+			    -Wclippy::unnecessary_safety_doc \
 			    -Wrustdoc::missing_crate_level_docs
 
 KBUILD_HOSTCFLAGS   := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
-- 
2.46.0

Re: [PATCH 06/19] rust: enable `clippy::unnecessary_safety_doc` lint

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.67.0, Clippy added the `unnecessary_safety_doc` lint [1],
> which is similar to `unnecessary_safety_comment`, but for `# Safety`
> sections, i.e. safety preconditions in the documentation.
>
> This is something that should not happen with our coding guidelines in
> mind. Thus enable the lint to have it machine-checked.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_safety_doc [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 06/19] rust: enable `clippy::unnecessary_safety_doc` lint

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.67.0, Clippy added the `unnecessary_safety_doc` lint [1],
> which is similar to `unnecessary_safety_comment`, but for `# Safety`
> sections, i.e. safety preconditions in the documentation.
>
> This is something that should not happen with our coding guidelines in
> mind. Thus enable the lint to have it machine-checked.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_safety_doc [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 07/19] rust: enable `clippy::ignored_unit_patterns` lint

[Miguel Ojeda]

In Rust 1.73.0, Clippy introduced the `ignored_unit_patterns` lint [1]:

> Matching with `()` explicitly instead of `_` outlines the fact that
> the pattern contains no data. Also it would detect a type change
> that `_` would ignore.

There is only a single case that requires a change:

    error: matching over `()` is more explicit
       --> rust/kernel/types.rs:176:45
        |
    176 |         ScopeGuard::new_with_data((), move |_| cleanup())
        |                                             ^ help: use `()` instead of `_`: `()`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns
        = note: requested on the command line with `-D clippy::ignored-unit-patterns`

Thus clean it up and enable the lint -- no functional change intended.

Link: https://rust-lang.github.io/rust-clippy/master/index.html#/ignored_unit_patterns [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile             | 1 +
 rust/kernel/types.rs | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index f903309db91b..cc1b01590227 100644
--- a/Makefile
+++ b/Makefile
@@ -452,6 +452,7 @@ export rust_common_flags := --edition=2021 \
 			    -Wunreachable_pub \
 			    -Wclippy::all \
 			    -Wclippy::dbg_macro \
+			    -Wclippy::ignored_unit_patterns \
 			    -Wclippy::mut_mut \
 			    -Wclippy::needless_bitwise_bool \
 			    -Wclippy::needless_continue \
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 6c2d5fa9bce3..4e03df725f3f 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -225,7 +225,7 @@ pub fn dismiss(mut self) -> T {
 impl ScopeGuard<(), fn(())> {
     /// Creates a new guarded object with the given cleanup function.
     pub fn new(cleanup: impl FnOnce()) -> ScopeGuard<(), impl FnOnce(())> {
-        ScopeGuard::new_with_data((), move |_| cleanup())
+        ScopeGuard::new_with_data((), move |()| cleanup())
     }
 }
 
-- 
2.46.0

Re: [PATCH 07/19] rust: enable `clippy::ignored_unit_patterns` lint

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.73.0, Clippy introduced the `ignored_unit_patterns` lint [1]:
>
> > Matching with `()` explicitly instead of `_` outlines the fact that
> > the pattern contains no data. Also it would detect a type change
> > that `_` would ignore.
>
> There is only a single case that requires a change:
>
>     error: matching over `()` is more explicit
>        --> rust/kernel/types.rs:176:45
>         |
>     176 |         ScopeGuard::new_with_data((), move |_| cleanup())
>         |                                             ^ help: use `()` instead of `_`: `()`
>         |
>         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns
>         = note: requested on the command line with `-D clippy::ignored-unit-patterns`
>
> Thus clean it up and enable the lint -- no functional change intended.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/ignored_unit_patterns [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 07/19] rust: enable `clippy::ignored_unit_patterns` lint

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.73.0, Clippy introduced the `ignored_unit_patterns` lint [1]:
>
> > Matching with `()` explicitly instead of `_` outlines the fact that
> > the pattern contains no data. Also it would detect a type change
> > that `_` would ignore.
>
> There is only a single case that requires a change:
>
>     error: matching over `()` is more explicit
>        --> rust/kernel/types.rs:176:45
>         |
>     176 |         ScopeGuard::new_with_data((), move |_| cleanup())
>         |                                             ^ help: use `()` instead of `_`: `()`
>         |
>         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns
>         = note: requested on the command line with `-D clippy::ignored-unit-patterns`
>
> Thus clean it up and enable the lint -- no functional change intended.
>
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/ignored_unit_patterns [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 08/19] rust: enable `rustdoc::unescaped_backticks` lint

[Miguel Ojeda]

In Rust 1.71.0, `rustdoc` added the `unescaped_backticks` lint, which
detects what are typically typos in Markdown formatting regarding inline
code [1], e.g. from the Rust standard library:

    /// ... to `deref`/`deref_mut`` must ...

    /// ... use [`from_mut`]`. Specifically, ...

It does not seem to have almost any false positives, from the experience
of enabling it in the Rust standard library [2], which will be checked
starting with Rust 1.82.0. The maintainers also confirmed it is ready
to be used.

Thus enable it.

Link: https://doc.rust-lang.org/rustdoc/lints.html#unescaped_backticks [1]
Link: https://github.com/rust-lang/rust/pull/128307 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile      | 3 ++-
 rust/Makefile | 5 ++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index cc1b01590227..fc66bac4b4f1 100644
--- a/Makefile
+++ b/Makefile
@@ -460,7 +460,8 @@ export rust_common_flags := --edition=2021 \
 			    -Wclippy::undocumented_unsafe_blocks \
 			    -Wclippy::unnecessary_safety_comment \
 			    -Wclippy::unnecessary_safety_doc \
-			    -Wrustdoc::missing_crate_level_docs
+			    -Wrustdoc::missing_crate_level_docs \
+			    -Wrustdoc::unescaped_backticks
 
 KBUILD_HOSTCFLAGS   := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
 		       $(HOSTCFLAGS) -I $(srctree)/scripts/include
diff --git a/rust/Makefile b/rust/Makefile
index e13d14ec5fe7..bc8ae3cc0537 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -61,7 +61,7 @@ alloc-cfgs = \
 quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
       cmd_rustdoc = \
 	OBJTREE=$(abspath $(objtree)) \
-	$(RUSTDOC) $(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) \
+	$(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags))) \
 		$(rustc_target_flags) -L$(objtree)/$(obj) \
 		-Zunstable-options --generate-link-to-definition \
 		--output $(rustdoc_output) \
@@ -98,6 +98,9 @@ rustdoc-macros: private rustc_target_flags = --crate-type proc-macro \
 rustdoc-macros: $(src)/macros/lib.rs FORCE
 	+$(call if_changed,rustdoc)
 
+# Starting with Rust 1.82.0, skipping `-Wrustdoc::unescaped_backticks` should
+# not be needed -- see https://github.com/rust-lang/rust/pull/128307.
+rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks
 rustdoc-core: private rustc_target_flags = $(core-cfgs)
 rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE
 	+$(call if_changed,rustdoc)
-- 
2.46.0

Re: [PATCH 08/19] rust: enable `rustdoc::unescaped_backticks` lint

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.71.0, `rustdoc` added the `unescaped_backticks` lint, which
> detects what are typically typos in Markdown formatting regarding inline
> code [1], e.g. from the Rust standard library:
>
>     /// ... to `deref`/`deref_mut`` must ...
>
>     /// ... use [`from_mut`]`. Specifically, ...
>
> It does not seem to have almost any false positives, from the experience
> of enabling it in the Rust standard library [2], which will be checked
> starting with Rust 1.82.0. The maintainers also confirmed it is ready
> to be used.
>
> Thus enable it.
>
> Link: https://doc.rust-lang.org/rustdoc/lints.html#unescaped_backticks [1]
> Link: https://github.com/rust-lang/rust/pull/128307 [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 08/19] rust: enable `rustdoc::unescaped_backticks` lint

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.71.0, `rustdoc` added the `unescaped_backticks` lint, which
> detects what are typically typos in Markdown formatting regarding inline
> code [1], e.g. from the Rust standard library:
>
>     /// ... to `deref`/`deref_mut`` must ...
>
>     /// ... use [`from_mut`]`. Specifically, ...
>
> It does not seem to have almost any false positives, from the experience
> of enabling it in the Rust standard library [2], which will be checked
> starting with Rust 1.82.0. The maintainers also confirmed it is ready
> to be used.
>
> Thus enable it.
>
> Link: https://doc.rust-lang.org/rustdoc/lints.html#unescaped_backticks [1]
> Link: https://github.com/rust-lang/rust/pull/128307 [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

> +# Starting with Rust 1.82.0, skipping `-Wrustdoc::unescaped_backticks` should
> +# not be needed -- see https://github.com/rust-lang/rust/pull/128307.
> +rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks

Maybe we should use something like FIXME(msrv) so things like this are
easy to find?

Re: [PATCH 08/19] rust: enable `rustdoc::unescaped_backticks` lint

[Miguel Ojeda]

On Sun, Sep 29, 2024 at 6:40 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> Maybe we should use something like FIXME(msrv) so things like this are
> easy to find?

It would be nice to standardize on something. As long as the version
number is there, I think it is OK even without a standard notation,
since it will be easy to find while grepping for cleanups on the
version upgrade.

Relatedly, Benno even proposed failing the build [1].

[1] https://lore.kernel.org/rust-for-linux/CANiq72=-bV_=TUoH6gLnPwTcxROBqyrCpOpbumki_S+po1TPhQ@mail.gmail.com/

Cheers,
Miguel

[PATCH 09/19] rust: init: remove unneeded `#[allow(clippy::disallowed_names)]`

[Miguel Ojeda]

These few cases, unlike others in the same file, did not need the `allow`.

Thus clean them up.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/init.rs | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 08b9d695c285..aec26a4decb1 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -87,7 +87,6 @@
 //! To declare an init macro/function you just return an [`impl PinInit<T, E>`]:
 //!
 //! ```rust
-//! # #![allow(clippy::disallowed_names)]
 //! # use kernel::{sync::Mutex, new_mutex, init::PinInit, try_pin_init};
 //! #[pin_data]
 //! struct DriverData {
@@ -368,7 +367,6 @@ macro_rules! stack_try_pin_init {
 /// The syntax is almost identical to that of a normal `struct` initializer:
 ///
 /// ```rust
-/// # #![allow(clippy::disallowed_names)]
 /// # use kernel::{init, pin_init, macros::pin_data, init::*};
 /// # use core::pin::Pin;
 /// #[pin_data]
@@ -413,7 +411,6 @@ macro_rules! stack_try_pin_init {
 /// To create an initializer function, simply declare it like this:
 ///
 /// ```rust
-/// # #![allow(clippy::disallowed_names)]
 /// # use kernel::{init, pin_init, init::*};
 /// # use core::pin::Pin;
 /// # #[pin_data]
@@ -468,7 +465,6 @@ macro_rules! stack_try_pin_init {
 /// They can also easily embed it into their own `struct`s:
 ///
 /// ```rust
-/// # #![allow(clippy::disallowed_names)]
 /// # use kernel::{init, pin_init, macros::pin_data, init::*};
 /// # use core::pin::Pin;
 /// # #[pin_data]
-- 
2.46.0

Re: [PATCH 09/19] rust: init: remove unneeded `#[allow(clippy::disallowed_names)]`

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> These few cases, unlike others in the same file, did not need the `allow`.
>
> Thus clean them up.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 09/19] rust: init: remove unneeded `#[allow(clippy::disallowed_names)]`

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> These few cases, unlike others in the same file, did not need the `allow`.
>
> Thus clean them up.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 10/19] rust: sync: remove unneeded `#[allow(clippy::non_send_fields_in_send_ty)]`

[Miguel Ojeda]

Rust 1.58.0 (before Rust was merged into the kernel) made Clippy's
`non_send_fields_in_send_ty` lint part of the `suspicious` lint group for
a brief window of time [1] until the minor version 1.58.1 got released
a week after, where the lint was moved back to `nursery`.

By that time, we had already upgraded to that Rust version, and thus we
had `allow`ed the lint here for `CondVar`.

Nowadays, Clippy's `non_send_fields_in_send_ty` would still trigger here
if it were enabled.

Moreover, if enabled, `Lock<T, B>` and `Task` would also require an
`allow`. Therefore, it does not seem like someone is actually enabling it
(in, e.g., a custom flags build).

Finally, the lint does not appear to have had major improvements since
then [2].

Thus remove the `allow` since it is unneeded.

Link: https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1581-2022-01-20 [1]
Link: https://github.com/rust-lang/rust-clippy/issues/8045 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/sync/condvar.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index 7e00048bf4b1..dec2e5ffc919 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -92,7 +92,6 @@ pub struct CondVar {
     _pin: PhantomPinned,
 }
 
-#[allow(clippy::non_send_fields_in_send_ty)]
 // SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on any thread.
 unsafe impl Send for CondVar {}
 
-- 
2.46.0

Re: [PATCH 10/19] rust: sync: remove unneeded `#[allow(clippy::non_send_fields_in_send_ty)]`

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Rust 1.58.0 (before Rust was merged into the kernel) made Clippy's
> `non_send_fields_in_send_ty` lint part of the `suspicious` lint group for
> a brief window of time [1] until the minor version 1.58.1 got released
> a week after, where the lint was moved back to `nursery`.
>
> By that time, we had already upgraded to that Rust version, and thus we
> had `allow`ed the lint here for `CondVar`.
>
> Nowadays, Clippy's `non_send_fields_in_send_ty` would still trigger here
> if it were enabled.
>
> Moreover, if enabled, `Lock<T, B>` and `Task` would also require an
> `allow`. Therefore, it does not seem like someone is actually enabling it
> (in, e.g., a custom flags build).
>
> Finally, the lint does not appear to have had major improvements since
> then [2].
>
> Thus remove the `allow` since it is unneeded.
>
> Link: https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1581-2022-01-20 [1]
> Link: https://github.com/rust-lang/rust-clippy/issues/8045 [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 10/19] rust: sync: remove unneeded `#[allow(clippy::non_send_fields_in_send_ty)]`

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Rust 1.58.0 (before Rust was merged into the kernel) made Clippy's
> `non_send_fields_in_send_ty` lint part of the `suspicious` lint group for
> a brief window of time [1] until the minor version 1.58.1 got released
> a week after, where the lint was moved back to `nursery`.
>
> By that time, we had already upgraded to that Rust version, and thus we
> had `allow`ed the lint here for `CondVar`.
>
> Nowadays, Clippy's `non_send_fields_in_send_ty` would still trigger here
> if it were enabled.
>
> Moreover, if enabled, `Lock<T, B>` and `Task` would also require an
> `allow`. Therefore, it does not seem like someone is actually enabling it
> (in, e.g., a custom flags build).
>
> Finally, the lint does not appear to have had major improvements since
> then [2].
>
> Thus remove the `allow` since it is unneeded.
>
> Link: https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1581-2022-01-20 [1]
> Link: https://github.com/rust-lang/rust-clippy/issues/8045 [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 11/19] rust: introduce `.clippy.toml`

[Miguel Ojeda]

Some Clippy lints can be configured/tweaked. We will use these knobs to
our advantage in later commits.

This is done via a configuration file, `.clippy.toml` [1]. The file is
currently unstable. This may be a problem in the future, but we can adapt
as needed. In addition, we proposed adding Clippy to the Rust CI's RFL
job [2], so we should be able to catch issues pre-merge.

Thus introduce the file.

Link: https://doc.rust-lang.org/clippy/configuration.html [1]
Link: https://github.com/rust-lang/rust/pull/128928 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 .clippy.toml | 1 +
 .gitignore   | 1 +
 MAINTAINERS  | 1 +
 Makefile     | 3 +++
 4 files changed, 6 insertions(+)
 create mode 100644 .clippy.toml

diff --git a/.clippy.toml b/.clippy.toml
new file mode 100644
index 000000000000..f66554cd5c45
--- /dev/null
+++ b/.clippy.toml
@@ -0,0 +1 @@
+# SPDX-License-Identifier: GPL-2.0
diff --git a/.gitignore b/.gitignore
index 7902adf4f7f1..907c782962d2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -102,6 +102,7 @@ modules.order
 # We don't want to ignore the following even if they are dot-files
 #
 !.clang-format
+!.clippy.toml
 !.cocciconfig
 !.editorconfig
 !.get_maintainer.ignore
diff --git a/MAINTAINERS b/MAINTAINERS
index 77b395476a80..cb63f1f97556 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19922,6 +19922,7 @@ B:	https://github.com/Rust-for-Linux/linux/issues
 C:	zulip://rust-for-linux.zulipchat.com
 P:	https://rust-for-linux.com/contributing
 T:	git https://github.com/Rust-for-Linux/linux.git rust-next
+F:	.clippy.toml
 F:	Documentation/rust/
 F:	rust/
 F:	samples/rust/
diff --git a/Makefile b/Makefile
index fc66bac4b4f1..234ab97de796 100644
--- a/Makefile
+++ b/Makefile
@@ -590,6 +590,9 @@ endif
 # Allows the usage of unstable features in stable compilers.
 export RUSTC_BOOTSTRAP := 1
 
+# Allows finding `.clippy.toml` in out-of-srctree builds.
+export CLIPPY_CONF_DIR := $(srctree)
+
 export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG
 export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN
 export HOSTRUSTC KBUILD_HOSTRUSTFLAGS
-- 
2.46.0

Re: [PATCH 11/19] rust: introduce `.clippy.toml`

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Some Clippy lints can be configured/tweaked. We will use these knobs to
> our advantage in later commits.
>
> This is done via a configuration file, `.clippy.toml` [1]. The file is
> currently unstable. This may be a problem in the future, but we can adapt
> as needed. In addition, we proposed adding Clippy to the Rust CI's RFL
> job [2], so we should be able to catch issues pre-merge.
>
> Thus introduce the file.
>
> Link: https://doc.rust-lang.org/clippy/configuration.html [1]
> Link: https://github.com/rust-lang/rust/pull/128928 [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 11/19] rust: introduce `.clippy.toml`

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Some Clippy lints can be configured/tweaked. We will use these knobs to
> our advantage in later commits.
>
> This is done via a configuration file, `.clippy.toml` [1]. The file is
> currently unstable. This may be a problem in the future, but we can adapt
> as needed. In addition, we proposed adding Clippy to the Rust CI's RFL
> job [2], so we should be able to catch issues pre-merge.
>
> Thus introduce the file.
>
> Link: https://doc.rust-lang.org/clippy/configuration.html [1]
> Link: https://github.com/rust-lang/rust/pull/128928 [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 12/19] rust: replace `clippy::dbg_macro` with `disallowed_macros`

[Miguel Ojeda]

Back when we used Rust 1.60.0 (before Rust was merged in the kernel),
we added `-Wclippy::dbg_macro` to the compilation flags. This worked
great with our custom `dbg!` macro (vendored from `std`, but slightly
modified to use the kernel printing facilities).

However, in the very next version, 1.61.0, it stopped working [1] since
the lint started to use a Rust diagnostic item rather than a path to find
the `dbg!` macro [1]. This behavior remains until the current nightly
(1.83.0).

Therefore, currently, the `dbg_macro` is not doing anything, which
explains why we can invoke `dbg!` in samples/rust/rust_print.rs`, as well
as why changing the `#[allow()]`s to `#[expect()]`s in `std_vendor.rs`
doctests does not work since they are not fulfilled.

One possible workaround is using `rustc_attrs` like the standard library
does. However, this is intended to be internal, and we just started
supporting several Rust compiler versions, so it is best to avoid it.

Therefore, instead, use `disallowed_macros`. It is a stable lint and
is more flexible (in that we can provide different macros), although
its diagnostic message(s) are not as nice as the specialized one (yet),
and does not allow to set different lint levels per macro/path [2].

In turn, this requires allowing the (intentional) `dbg!` use in the
sample, as one would have expected.

Finally, in a single case, the `allow` is fixed to be an inner attribute,
since otherwise it was not being applied.

Link: https://github.com/rust-lang/rust-clippy/issues/11303 [1]
Link: https://github.com/rust-lang/rust-clippy/issues/11307 [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 .clippy.toml               |  6 ++++++
 Makefile                   |  1 -
 rust/kernel/std_vendor.rs  | 10 +++++-----
 samples/rust/rust_print.rs |  1 +
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/.clippy.toml b/.clippy.toml
index f66554cd5c45..ad9f804fb677 100644
--- a/.clippy.toml
+++ b/.clippy.toml
@@ -1 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
+
+disallowed-macros = [
+    # The `clippy::dbg_macro` lint only works with `std::dbg!`, thus we simulate
+    # it here, see: https://github.com/rust-lang/rust-clippy/issues/11303.
+    { path = "kernel::dbg", reason = "the `dbg!` macro is intended as a debugging tool" },
+]
diff --git a/Makefile b/Makefile
index 234ab97de796..f236dd5fb6d9 100644
--- a/Makefile
+++ b/Makefile
@@ -451,7 +451,6 @@ export rust_common_flags := --edition=2021 \
 			    -Wrust_2018_idioms \
 			    -Wunreachable_pub \
 			    -Wclippy::all \
-			    -Wclippy::dbg_macro \
 			    -Wclippy::ignored_unit_patterns \
 			    -Wclippy::mut_mut \
 			    -Wclippy::needless_bitwise_bool \
diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
index 67bf9d37ddb5..085b23312c65 100644
--- a/rust/kernel/std_vendor.rs
+++ b/rust/kernel/std_vendor.rs
@@ -14,7 +14,7 @@
 ///
 /// ```rust
 /// let a = 2;
-/// # #[allow(clippy::dbg_macro)]
+/// # #[allow(clippy::disallowed_macros)]
 /// let b = dbg!(a * 2) + 1;
 /// //      ^-- prints: [src/main.rs:2] a * 2 = 4
 /// assert_eq!(b, 5);
@@ -52,7 +52,7 @@
 /// With a method call:
 ///
 /// ```rust
-/// # #[allow(clippy::dbg_macro)]
+/// # #[allow(clippy::disallowed_macros)]
 /// fn foo(n: usize) {
 ///     if dbg!(n.checked_sub(4)).is_some() {
 ///         // ...
@@ -71,7 +71,7 @@
 /// Naive factorial implementation:
 ///
 /// ```rust
-/// # #[allow(clippy::dbg_macro)]
+/// # #[allow(clippy::disallowed_macros)]
 /// # {
 /// fn factorial(n: u32) -> u32 {
 ///     if dbg!(n <= 1) {
@@ -118,7 +118,7 @@
 /// a tuple (and return it, too):
 ///
 /// ```
-/// # #[allow(clippy::dbg_macro)]
+/// # #![allow(clippy::disallowed_macros)]
 /// assert_eq!(dbg!(1usize, 2u32), (1, 2));
 /// ```
 ///
@@ -127,7 +127,7 @@
 /// invocations. You can use a 1-tuple directly if you need one:
 ///
 /// ```
-/// # #[allow(clippy::dbg_macro)]
+/// # #[allow(clippy::disallowed_macros)]
 /// # {
 /// assert_eq!(1, dbg!(1u32,)); // trailing comma ignored
 /// assert_eq!((1,), dbg!((1u32,))); // 1-tuple
diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs
index 6eabb0d79ea3..ed1137ab2018 100644
--- a/samples/rust/rust_print.rs
+++ b/samples/rust/rust_print.rs
@@ -15,6 +15,7 @@
 
 struct RustPrint;
 
+#[allow(clippy::disallowed_macros)]
 fn arc_print() -> Result {
     use kernel::sync::*;
 
-- 
2.46.0

Re: [PATCH 12/19] rust: replace `clippy::dbg_macro` with `disallowed_macros`

[Geert Stappers]

On Wed, Sep 04, 2024 at 10:43:40PM +0200, Miguel Ojeda wrote:
> diff --git a/.clippy.toml b/.clippy.toml
> index f66554cd5c45..ad9f804fb677 100644
> --- a/.clippy.toml
> +++ b/.clippy.toml
> @@ -1 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
> +
> +disallowed-macros = [
> +    # The `clippy::dbg_macro` lint only works with `std::dbg!`, thus we simulate
> +    # it here, see: https://github.com/rust-lang/rust-clippy/issues/11303.
> +    { path = "kernel::dbg", reason = "the `dbg!` macro is intended as a debugging tool" },
> +]
> diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
> index 67bf9d37ddb5..085b23312c65 100644
> --- a/rust/kernel/std_vendor.rs
> +++ b/rust/kernel/std_vendor.rs
> @@ -14,7 +14,7 @@
> -/// # #[allow(clippy::dbg_macro)]
> +/// # #[allow(clippy::disallowed_macros)]
> @@ -52,7 +52,7 @@
> -/// # #[allow(clippy::dbg_macro)]
> +/// # #[allow(clippy::disallowed_macros)]
> @@ -71,7 +71,7 @@
> -/// # #[allow(clippy::dbg_macro)]
> +/// # #[allow(clippy::disallowed_macros)]
> @@ -118,7 +118,7 @@
>  /// a tuple (and return it, too):
>  ///
>  /// ```
> -/// # #[allow(clippy::dbg_macro)]
> +/// # #![allow(clippy::disallowed_macros)]
>  /// assert_eq!(dbg!(1usize, 2u32), (1, 2));
>  /// ```
>  ///

For what it is worth, the exclamation mark did surprise me.


> @@ -127,7 +127,7 @@
>  ///
>  /// ```
> -/// # #[allow(clippy::dbg_macro)]
> +/// # #[allow(clippy::disallowed_macros)]
>  /// # {
>  /// assert_eq!(1, dbg!(1u32,)); // trailing comma ignored
>  /// assert_eq!((1,), dbg!((1u32,))); // 1-tuple


Groeten
Geert Stappers
-- 
Silence is hard to parse

Re: [PATCH 12/19] rust: replace `clippy::dbg_macro` with `disallowed_macros`

[Gary Guo]

On Thu, 5 Sep 2024 07:20:46 +0200
Geert Stappers <stappers@stappers.nl> wrote:

> On Wed, Sep 04, 2024 at 10:43:40PM +0200, Miguel Ojeda wrote:
> > diff --git a/.clippy.toml b/.clippy.toml
> > index f66554cd5c45..ad9f804fb677 100644
> > --- a/.clippy.toml
> > +++ b/.clippy.toml
> > @@ -1 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0
> > +
> > +disallowed-macros = [
> > +    # The `clippy::dbg_macro` lint only works with `std::dbg!`, thus we simulate
> > +    # it here, see: https://github.com/rust-lang/rust-clippy/issues/11303.
> > +    { path = "kernel::dbg", reason = "the `dbg!` macro is intended as a debugging tool" },
> > +]
> > diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
> > index 67bf9d37ddb5..085b23312c65 100644
> > --- a/rust/kernel/std_vendor.rs
> > +++ b/rust/kernel/std_vendor.rs
> > @@ -14,7 +14,7 @@
> > -/// # #[allow(clippy::dbg_macro)]
> > +/// # #[allow(clippy::disallowed_macros)]
> > @@ -52,7 +52,7 @@
> > -/// # #[allow(clippy::dbg_macro)]
> > +/// # #[allow(clippy::disallowed_macros)]
> > @@ -71,7 +71,7 @@
> > -/// # #[allow(clippy::dbg_macro)]
> > +/// # #[allow(clippy::disallowed_macros)]
> > @@ -118,7 +118,7 @@
> >  /// a tuple (and return it, too):
> >  ///
> >  /// ```
> > -/// # #[allow(clippy::dbg_macro)]
> > +/// # #![allow(clippy::disallowed_macros)]
> >  /// assert_eq!(dbg!(1usize, 2u32), (1, 2));
> >  /// ```
> >  ///  
> 
> For what it is worth, the exclamation mark did surprise me.

`#[]` would apply to the next item/statement, and `#![]` would apply to
the surrouding scope. In this case there's just a single statement so
they should be equivalent.

Miguel, is this change from `#[]` to `#![]` intentional?

Best,
Gary

> 
> 
> > @@ -127,7 +127,7 @@
> >  ///
> >  /// ```
> > -/// # #[allow(clippy::dbg_macro)]
> > +/// # #[allow(clippy::disallowed_macros)]
> >  /// # {
> >  /// assert_eq!(1, dbg!(1u32,)); // trailing comma ignored
> >  /// assert_eq!((1,), dbg!((1u32,))); // 1-tuple  
> 
> 
> Groeten
> Geert Stappers

Re: [PATCH 12/19] rust: replace `clippy::dbg_macro` with `disallowed_macros`

[Miguel Ojeda]

On Sun, Sep 15, 2024 at 1:10 AM Gary Guo <gary@garyguo.net> wrote:
>
> `#[]` would apply to the next item/statement, and `#![]` would apply to
> the surrouding scope. In this case there's just a single statement so
> they should be equivalent.
>
> Miguel, is this change from `#[]` to `#![]` intentional?

Yeah, it is intentional -- it is what the last paragraph in the commit
message refers to.

I think this is e.g. https://github.com/rust-lang/rust/issues/87391
and https://github.com/rust-lang/rust-clippy/issues/10355:

    note: the built-in attribute `expect` will be ignored, since it's
applied to the macro invocation `assert_eq`

In addition, `disallowed_macros` also has some false negatives I
noticed back when I started playing with this:
https://github.com/rust-lang/rust-clippy/issues/11431

Cheers,
Miguel

[PATCH 13/19] rust: rbtree: fix `SAFETY` comments that should be `# Safety` sections

[Miguel Ojeda]

The tag `SAFETY` is used for safety comments, i.e. `// SAFETY`, while a
`Safety` section is used for safety preconditions in code documentation,
i.e. `/// # Safety`.

Fix the three instances recently added in `rbtree` that Clippy would
have normally caught in a public item, so that we can enable checking
of private items in the next commit.

Fixes: 98c14e40e07a ("rust: rbtree: add cursor")
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/rbtree.rs | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
index 48ceb9560bf5..48e552799e17 100644
--- a/rust/kernel/rbtree.rs
+++ b/rust/kernel/rbtree.rs
@@ -884,7 +884,8 @@ fn get_neighbor_raw(&self, direction: Direction) -> Option<NonNull<bindings::rb_
         NonNull::new(neighbor)
     }
 
-    /// SAFETY:
+    /// # Safety
+    ///
     /// - `node` must be a valid pointer to a node in an [`RBTree`].
     /// - The caller has immutable access to `node` for the duration of 'b.
     unsafe fn to_key_value<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b V) {
@@ -894,7 +895,8 @@ unsafe fn to_key_value<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b V) {
         (k, unsafe { &*v })
     }
 
-    /// SAFETY:
+    /// # Safety
+    ///
     /// - `node` must be a valid pointer to a node in an [`RBTree`].
     /// - The caller has mutable access to `node` for the duration of 'b.
     unsafe fn to_key_value_mut<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b mut V) {
@@ -904,7 +906,8 @@ unsafe fn to_key_value_mut<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b
         (k, unsafe { &mut *v })
     }
 
-    /// SAFETY:
+    /// # Safety
+    ///
     /// - `node` must be a valid pointer to a node in an [`RBTree`].
     /// - The caller has immutable access to the key for the duration of 'b.
     unsafe fn to_key_value_raw<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, *mut V) {
-- 
2.46.0

Re: [PATCH 13/19] rust: rbtree: fix `SAFETY` comments that should be `# Safety` sections

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> The tag `SAFETY` is used for safety comments, i.e. `// SAFETY`, while a
> `Safety` section is used for safety preconditions in code documentation,
> i.e. `/// # Safety`.
>
> Fix the three instances recently added in `rbtree` that Clippy would
> have normally caught in a public item, so that we can enable checking
> of private items in the next commit.
>
> Fixes: 98c14e40e07a ("rust: rbtree: add cursor")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 13/19] rust: rbtree: fix `SAFETY` comments that should be `# Safety` sections

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> The tag `SAFETY` is used for safety comments, i.e. `// SAFETY`, while a
> `Safety` section is used for safety preconditions in code documentation,
> i.e. `/// # Safety`.
>
> Fix the three instances recently added in `rbtree` that Clippy would
> have normally caught in a public item, so that we can enable checking
> of private items in the next commit.
>
> Fixes: 98c14e40e07a ("rust: rbtree: add cursor")
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 14/19] rust: provide proper code documentation titles

[Miguel Ojeda]

Rust 1.82.0's Clippy is introducing [1][2] a new warn-by-default lint,
`too_long_first_doc_paragraph` [3], which is intended to catch titles
of code documentation items that are too long (likely because no title
was provided and the item documentation starts with a paragraph).

This lint does not currently trigger anywhere, but it does detect a couple
cases we had in private cases if checking for private items gets enabled
(which we will do in the next commit):

    error: first doc comment paragraph is too long
      --> rust/kernel/init/__internal.rs:18:1
       |
    18 | / /// This is the module-internal type implementing `PinInit` and `Init`. It is unsafe to create this
    19 | | /// type, since the closure needs to fulfill the same safety requirement as the
    20 | | /// `__pinned_init`/`__init` functions.
       | |_
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
       = note: `-D clippy::too-long-first-doc-paragraph` implied by `-D warnings`
       = help: to override `-D warnings` add `#[allow(clippy::too_long_first_doc_paragraph)]`

    error: first doc comment paragraph is too long
     --> rust/kernel/sync/arc/std_vendor.rs:3:1
      |
    3 | / //! The contents of this file come from the Rust standard library, hosted in
    4 | | //! the <https://github.com/rust-lang/rust> repository, licensed under
    5 | | //! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
    6 | | //! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
      | |_
      |
      = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph

Thus clean the two instances we hit and enable the lint.

In addition, since we have a second `std_vendor.rs` file with a similar
header, do the same there too (even if that one does not trigger the lint,
because it is `doc(hidden)`).

Link: https://github.com/rust-lang/rust/pull/129531 [1]
Link: https://github.com/rust-lang/rust-clippy/pull/12993 [2]
Link: https://rust-lang.github.io/rust-clippy/master/index.html#/too_long_first_doc_paragraph [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/init/__internal.rs     | 7 ++++---
 rust/kernel/std_vendor.rs          | 2 ++
 rust/kernel/sync/arc/std_vendor.rs | 2 ++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
index 29f4fd00df3d..163eb072f296 100644
--- a/rust/kernel/init/__internal.rs
+++ b/rust/kernel/init/__internal.rs
@@ -15,9 +15,10 @@
 /// [this table]: https://doc.rust-lang.org/nomicon/phantom-data.html#table-of-phantomdata-patterns
 pub(super) type Invariant<T> = PhantomData<fn(*mut T) -> *mut T>;
 
-/// This is the module-internal type implementing `PinInit` and `Init`. It is unsafe to create this
-/// type, since the closure needs to fulfill the same safety requirement as the
-/// `__pinned_init`/`__init` functions.
+/// Module-internal type implementing `PinInit` and `Init`.
+///
+/// It is unsafe to create this type, since the closure needs to fulfill the same safety
+/// requirement as the `__pinned_init`/`__init` functions.
 pub(crate) struct InitClosure<F, T: ?Sized, E>(pub(crate) F, pub(crate) Invariant<(E, T)>);
 
 // SAFETY: While constructing the `InitClosure`, the user promised that it upholds the
diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
index 085b23312c65..d59e4cf4b252 100644
--- a/rust/kernel/std_vendor.rs
+++ b/rust/kernel/std_vendor.rs
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
+//! Rust standard library vendored code.
+//!
 //! The contents of this file come from the Rust standard library, hosted in
 //! the <https://github.com/rust-lang/rust> repository, licensed under
 //! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
diff --git a/rust/kernel/sync/arc/std_vendor.rs b/rust/kernel/sync/arc/std_vendor.rs
index a66a0c2831b3..11b3f4ecca5f 100644
--- a/rust/kernel/sync/arc/std_vendor.rs
+++ b/rust/kernel/sync/arc/std_vendor.rs
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
+//! Rust standard library vendored code.
+//!
 //! The contents of this file come from the Rust standard library, hosted in
 //! the <https://github.com/rust-lang/rust> repository, licensed under
 //! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
-- 
2.46.0

Re: [PATCH 14/19] rust: provide proper code documentation titles

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Rust 1.82.0's Clippy is introducing [1][2] a new warn-by-default lint,
> `too_long_first_doc_paragraph` [3], which is intended to catch titles
> of code documentation items that are too long (likely because no title
> was provided and the item documentation starts with a paragraph).
>
> This lint does not currently trigger anywhere, but it does detect a couple
> cases we had in private cases if checking for private items gets enabled
> (which we will do in the next commit):
>
>     error: first doc comment paragraph is too long
>       --> rust/kernel/init/__internal.rs:18:1
>        |
>     18 | / /// This is the module-internal type implementing `PinInit` and `Init`. It is unsafe to create this
>     19 | | /// type, since the closure needs to fulfill the same safety requirement as the
>     20 | | /// `__pinned_init`/`__init` functions.
>        | |_
>        |
>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
>        = note: `-D clippy::too-long-first-doc-paragraph` implied by `-D warnings`
>        = help: to override `-D warnings` add `#[allow(clippy::too_long_first_doc_paragraph)]`
>
>     error: first doc comment paragraph is too long
>      --> rust/kernel/sync/arc/std_vendor.rs:3:1
>       |
>     3 | / //! The contents of this file come from the Rust standard library, hosted in
>     4 | | //! the <https://github.com/rust-lang/rust> repository, licensed under
>     5 | | //! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
>     6 | | //! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
>       | |_
>       |
>       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
>
> Thus clean the two instances we hit and enable the lint.
>
> In addition, since we have a second `std_vendor.rs` file with a similar
> header, do the same there too (even if that one does not trigger the lint,
> because it is `doc(hidden)`).
>
> Link: https://github.com/rust-lang/rust/pull/129531 [1]
> Link: https://github.com/rust-lang/rust-clippy/pull/12993 [2]
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/too_long_first_doc_paragraph [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 14/19] rust: provide proper code documentation titles

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Rust 1.82.0's Clippy is introducing [1][2] a new warn-by-default lint,
> `too_long_first_doc_paragraph` [3], which is intended to catch titles
> of code documentation items that are too long (likely because no title
> was provided and the item documentation starts with a paragraph).
>
> This lint does not currently trigger anywhere, but it does detect a couple
> cases we had in private cases if checking for private items gets enabled
> (which we will do in the next commit):

I think some words got flipped around, should this say something like
"...a couple private cases we had in case checking..."?

>     error: first doc comment paragraph is too long
>       --> rust/kernel/init/__internal.rs:18:1
>        |
>     18 | / /// This is the module-internal type implementing `PinInit` and `Init`. It is unsafe to create this
>     19 | | /// type, since the closure needs to fulfill the same safety requirement as the
>     20 | | /// `__pinned_init`/`__init` functions.
>        | |_
>        |
>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
>        = note: `-D clippy::too-long-first-doc-paragraph` implied by `-D warnings`
>        = help: to override `-D warnings` add `#[allow(clippy::too_long_first_doc_paragraph)]`
>
>     error: first doc comment paragraph is too long
>      --> rust/kernel/sync/arc/std_vendor.rs:3:1
>       |
>     3 | / //! The contents of this file come from the Rust standard library, hosted in
>     4 | | //! the <https://github.com/rust-lang/rust> repository, licensed under
>     5 | | //! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
>     6 | | //! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
>       | |_
>       |
>       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
>
> Thus clean the two instances we hit and enable the lint.
>
> In addition, since we have a second `std_vendor.rs` file with a similar
> header, do the same there too (even if that one does not trigger the lint,
> because it is `doc(hidden)`).
>
> Link: https://github.com/rust-lang/rust/pull/129531 [1]
> Link: https://github.com/rust-lang/rust-clippy/pull/12993 [2]
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/too_long_first_doc_paragraph [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [PATCH 14/19] rust: provide proper code documentation titles

[Miguel Ojeda]

On Sun, Sep 29, 2024 at 6:56 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> I think some words got flipped around, should this say something like
> "...a couple private cases we had in case checking..."?

Nice catch, fixed in the version I am will apply -- thanks!

Cheers,
Miguel

[PATCH 15/19] rust: enable Clippy's `check-private-items`

[Miguel Ojeda]

In Rust 1.76.0, Clippy added the `check-private-items` lint configuration
option. When turned on (the default is off), it makes several lints
check private items as well.

In our case, it affects two lints we have enabled [1]:
`missing_safety_doc` and `unnecessary_safety_doc`.

It also seems to affect the new `too_long_first_doc_paragraph` lint [2],
even though the documentation does not mention it.

Thus allow the few instances remaining we currently hit and enable
the lint.

Link: https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#check-private-items [1]
Link: https://rust-lang.github.io/rust-clippy/master/index.html#/too_long_first_doc_paragraph [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 .clippy.toml                   | 2 ++
 rust/kernel/init.rs            | 1 +
 rust/kernel/init/__internal.rs | 2 ++
 rust/kernel/init/macros.rs     | 1 +
 rust/kernel/print.rs           | 1 +
 5 files changed, 7 insertions(+)

diff --git a/.clippy.toml b/.clippy.toml
index ad9f804fb677..e4c4eef10b28 100644
--- a/.clippy.toml
+++ b/.clippy.toml
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
+check-private-items = true
+
 disallowed-macros = [
     # The `clippy::dbg_macro` lint only works with `std::dbg!`, thus we simulate
     # it here, see: https://github.com/rust-lang/rust-clippy/issues/11303.
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index aec26a4decb1..10ec90a5f5d8 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -125,6 +125,7 @@
 //! use core::{ptr::addr_of_mut, marker::PhantomPinned, pin::Pin};
 //! # mod bindings {
 //! #     #![allow(non_camel_case_types)]
+//! #     #![allow(clippy::missing_safety_doc)]
 //! #     pub struct foo;
 //! #     pub unsafe fn init_foo(_ptr: *mut foo) {}
 //! #     pub unsafe fn destroy_foo(_ptr: *mut foo) {}
diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
index 163eb072f296..549ae227c2ea 100644
--- a/rust/kernel/init/__internal.rs
+++ b/rust/kernel/init/__internal.rs
@@ -54,6 +54,7 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
 pub unsafe trait HasPinData {
     type PinData: PinData;
 
+    #[allow(clippy::missing_safety_doc)]
     unsafe fn __pin_data() -> Self::PinData;
 }
 
@@ -83,6 +84,7 @@ fn make_closure<F, O, E>(self, f: F) -> F
 pub unsafe trait HasInitData {
     type InitData: InitData;
 
+    #[allow(clippy::missing_safety_doc)]
     unsafe fn __init_data() -> Self::InitData;
 }
 
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 93af8f3d8d4d..8733ba3834ab 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -989,6 +989,7 @@ fn drop(&mut self) {
         //
         // The functions are `unsafe` to prevent accidentally calling them.
         #[allow(dead_code)]
+        #[allow(clippy::missing_safety_doc)]
         impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
         where $($whr)*
         {
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index fe53fc469c4f..45af17095a24 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -14,6 +14,7 @@
 use crate::str::RawFormatter;
 
 // Called from `vsprintf` with format specifier `%pA`.
+#[allow(clippy::missing_safety_doc)]
 #[no_mangle]
 unsafe extern "C" fn rust_fmt_argument(
     buf: *mut c_char,
-- 
2.46.0

Re: [PATCH 15/19] rust: enable Clippy's `check-private-items`

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.76.0, Clippy added the `check-private-items` lint configuration
> option. When turned on (the default is off), it makes several lints
> check private items as well.
>
> In our case, it affects two lints we have enabled [1]:
> `missing_safety_doc` and `unnecessary_safety_doc`.
>
> It also seems to affect the new `too_long_first_doc_paragraph` lint [2],
> even though the documentation does not mention it.
>
> Thus allow the few instances remaining we currently hit and enable
> the lint.
>
> Link: https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#check-private-items [1]
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/too_long_first_doc_paragraph [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 15/19] rust: enable Clippy's `check-private-items`

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust 1.76.0, Clippy added the `check-private-items` lint configuration
> option. When turned on (the default is off), it makes several lints
> check private items as well.
>
> In our case, it affects two lints we have enabled [1]:
> `missing_safety_doc` and `unnecessary_safety_doc`.
>
> It also seems to affect the new `too_long_first_doc_paragraph` lint [2],
> even though the documentation does not mention it.
>
> Thus allow the few instances remaining we currently hit and enable
> the lint.
>
> Link: https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#check-private-items [1]
> Link: https://rust-lang.github.io/rust-clippy/master/index.html#/too_long_first_doc_paragraph [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 16/19] Documentation: rust: add coding guidelines on lints

[Miguel Ojeda]

In the C side, disabling diagnostics locally, i.e. within the source code,
is rare (at least in the kernel). Sometimes warnings are manipulated
via the flags at the translation unit level, but that is about it.

In Rust, it is easier to change locally the "level" of lints
(e.g. allowing them locally). In turn, this means it is easier to
globally enable more lints that may trigger a few false positives here
and there that need to be allowed ocally, but that generally can spot
issues or bugs.

Thus document this.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/rust/coding-guidelines.rst | 29 ++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst
index 05542840b16c..185d3b51117d 100644
--- a/Documentation/rust/coding-guidelines.rst
+++ b/Documentation/rust/coding-guidelines.rst
@@ -227,3 +227,32 @@ The equivalent in Rust may look like (ignoring documentation):
 That is, the equivalent of ``GPIO_LINE_DIRECTION_IN`` would be referred to as
 ``gpio::LineDirection::In``. In particular, it should not be named
 ``gpio::gpio_line_direction::GPIO_LINE_DIRECTION_IN``.
+
+
+Lints
+-----
+
+In Rust, it is possible to ``allow`` particular warnings (diagnostics, lints)
+locally, making the compiler ignore instances of a given warning within a given
+function, module, block, etc.
+
+It is similar to ``#pragma GCC diagnostic push`` + ``ignored`` + ``pop`` in C:
+
+.. code-block:: c
+
+	#pragma GCC diagnostic push
+	#pragma GCC diagnostic ignored "-Wunused-function"
+	static void f(void) {}
+	#pragma GCC diagnostic pop
+
+But way less verbose:
+
+.. code-block:: rust
+
+	#[allow(dead_code)]
+	fn f() {}
+
+By that virtue, it makes it possible to comfortably enable more diagnostics by
+default (i.e. outside ``W=`` levels). In particular, those that may have some
+false positives but that are otherwise quite useful to keep enabled to catch
+potential mistakes.
-- 
2.46.0

Re: [PATCH 16/19] Documentation: rust: add coding guidelines on lints

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In the C side, disabling diagnostics locally, i.e. within the source code,
> is rare (at least in the kernel). Sometimes warnings are manipulated
> via the flags at the translation unit level, but that is about it.
>
> In Rust, it is easier to change locally the "level" of lints
> (e.g. allowing them locally). In turn, this means it is easier to
> globally enable more lints that may trigger a few false positives here
> and there that need to be allowed ocally, but that generally can spot
> issues or bugs.
>
> Thus document this.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Wow, does C really not have an easier way to do it?

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 16/19] Documentation: rust: add coding guidelines on lints

[Miguel Ojeda]

On Thu, Sep 5, 2024 at 10:16 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Wow, does C really not have an easier way to do it?

Yeah, it would be nice to get a similar system in C.

There are targeted attributes that can annotate certain things, like
`[[maybe_unused]]` in C23 (and vendor attributes too like our
`__maybe_unused` macro), so `-Wunused-function` is not really the best
example in that sense -- I will think of a better one (it was nice to
use the same as in the other examples I wrote for `expect` later on,
which is why I used it).

But, as far as I am aware, there is no way to handle lints (and levels
and so on) in a simple and consistent way like Rust does.

Cheers,
Miguel

Re: [PATCH 16/19] Documentation: rust: add coding guidelines on lints

[comex]

> On Sep 5, 2024, at 2:45 AM, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> 
> On Thu, Sep 5, 2024 at 10:16 AM Alice Ryhl <aliceryhl@google.com> wrote:
>> 
>> Wow, does C really not have an easier way to do it?
> 
> Yeah, it would be nice to get a similar system in C.
> 
> There are targeted attributes that can annotate certain things, like
> `[[maybe_unused]]` in C23 (and vendor attributes too like our
> `__maybe_unused` macro), so `-Wunused-function` is not really the best
> example in that sense -- I will think of a better one (it was nice to
> use the same as in the other examples I wrote for `expect` later on,
> which is why I used it).
> 
> But, as far as I am aware, there is no way to handle lints (and levels
> and so on) in a simple and consistent way like Rust does.

You can always hide the pragmas behind a macro:

https://gcc.godbolt.org/z/WTEaYWW8c

It’s not perfect, because warning names sometimes differ between GCC and Clang, among other reasons.

Re: [PATCH 16/19] Documentation: rust: add coding guidelines on lints

[Miguel Ojeda]

On Sun, Sep 8, 2024 at 12:22 AM comex <comexk@gmail.com> wrote:
>
> You can always hide the pragmas behind a macro:
>
> https://gcc.godbolt.org/z/WTEaYWW8c
>
> It’s not perfect, because warning names sometimes differ between GCC and Clang, among other reasons.

That could be an interesting option for some projects, but yeah, as
you say, I think it is far from ideal. It breaks `clang-format`,
wrapping functions/structs/... would look weird, and generally the
syntax gets on the way of the "normal" code (so even for statements
one would probably want to keep the start and end on their own lines).

Cheers,
Miguel

Re: [PATCH 16/19] Documentation: rust: add coding guidelines on lints

[Miguel Ojeda]

On Thu, Sep 5, 2024 at 11:45 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> example in that sense -- I will think of a better one (it was nice to
> use the same as in the other examples I wrote for `expect` later on,
> which is why I used it).

I added a footnote in the version I am applying to be able to keep
using the same example (lint) everywhere.

Cheers,
Miguel

Re: [PATCH 16/19] Documentation: rust: add coding guidelines on lints

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In the C side, disabling diagnostics locally, i.e. within the source code,
> is rare (at least in the kernel). Sometimes warnings are manipulated
> via the flags at the translation unit level, but that is about it.
>
> In Rust, it is easier to change locally the "level" of lints
> (e.g. allowing them locally). In turn, this means it is easier to
> globally enable more lints that may trigger a few false positives here
> and there that need to be allowed ocally, but that generally can spot
> issues or bugs.

s/ocally/locally

> Thus document this.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/rust/coding-guidelines.rst | 29 ++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst
> index 05542840b16c..185d3b51117d 100644
> --- a/Documentation/rust/coding-guidelines.rst
> +++ b/Documentation/rust/coding-guidelines.rst
> @@ -227,3 +227,32 @@ The equivalent in Rust may look like (ignoring documentation):
>  That is, the equivalent of ``GPIO_LINE_DIRECTION_IN`` would be referred to as
>  ``gpio::LineDirection::In``. In particular, it should not be named
>  ``gpio::gpio_line_direction::GPIO_LINE_DIRECTION_IN``.
> +
> +
> +Lints
> +-----
> +
> +In Rust, it is possible to ``allow`` particular warnings (diagnostics, lints)
> +locally, making the compiler ignore instances of a given warning within a given
> +function, module, block, etc.
> +
> +It is similar to ``#pragma GCC diagnostic push`` + ``ignored`` + ``pop`` in C:
> +
> +.. code-block:: c
> +
> +       #pragma GCC diagnostic push
> +       #pragma GCC diagnostic ignored "-Wunused-function"
> +       static void f(void) {}
> +       #pragma GCC diagnostic pop
> +
> +But way less verbose:
> +
> +.. code-block:: rust
> +
> +       #[allow(dead_code)]
> +       fn f() {}
> +
> +By that virtue, it makes it possible to comfortably enable more diagnostics by
> +default (i.e. outside ``W=`` levels). In particular, those that may have some
> +false positives but that are otherwise quite useful to keep enabled to catch
> +potential mistakes.

It may be good to link to the docs on this,
https://doc.rust-lang.org/stable/reference/attributes/diagnostics.html.
Either way:

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [PATCH 16/19] Documentation: rust: add coding guidelines on lints

[Miguel Ojeda]

On Sun, Sep 29, 2024 at 7:03 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> s/ocally/locally

Fixed in the version I am applying -- thanks!

> It may be good to link to the docs on this,
> https://doc.rust-lang.org/stable/reference/attributes/diagnostics.html.

Done too!

Cheers,
Miguel

[PATCH 17/19] rust: start using the `#[expect(...)]` attribute

[Miguel Ojeda]

In Rust, it is possible to `allow` particular warnings (diagnostics,
lints) locally, making the compiler ignore instances of a given warning
within a given function, module, block, etc.

It is similar to `#pragma GCC diagnostic push` + `ignored` + `pop` in C:

    #pragma GCC diagnostic push
    #pragma GCC diagnostic ignored "-Wunused-function"
    static void f(void) {}
    #pragma GCC diagnostic pop

But way less verbose:

    #[allow(dead_code)]
    fn f() {}

By that virtue, it makes it possible to comfortably enable more
diagnostics by default (i.e. outside `W=` levels) that may have some
false positives but that are otherwise quite useful to keep enabled to
catch potential mistakes.

The `#[expect(...)]` attribute [1] takes this further, and makes the
compiler warn if the diagnostic was _not_ produced. For instance, the
following will ensure that, when `f()` is called somewhere, we will have
to remove the attribute:

    #[expect(dead_code)]
    fn f() {}

If we do not, we get a warning from the compiler:

    warning: this lint expectation is unfulfilled
     --> x.rs:3:10
      |
    3 | #[expect(dead_code)]
      |          ^^^^^^^^^
      |
      = note: `#[warn(unfulfilled_lint_expectations)]` on by default

This means that `expect`s do not get forgotten when they are not needed.

See the next commit for more details, nuances on its usage and
documentation on the feature.

The attribute requires the `lint_reasons` [2] unstable feature, but it
is becoming stable in 1.81.0 (to be released on 2024-09-05) and it has
already been useful to clean things up in this patch series, finding
cases where the `allow`s should not have been there.

Thus, enable `lint_reasons` and convert some of our `allow`s to `expect`s
where possible.

This feature was also an example of the ongoing collaboration between
Rust and the kernel -- we tested it in the kernel early on and found an
issue that was quickly resolved [3].

Cc: Fridtjof Stoldt <xfrednet@gmail.com>
Cc: Urgau <urgau@numericable.fr>
Link: https://rust-lang.github.io/rfcs/2383-lint-reasons.html#expect-lint-attribute [1]
Link: https://github.com/rust-lang/rust/issues/54503 [2]
Link: https://github.com/rust-lang/rust/issues/114557 [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/error.rs           |  2 +-
 rust/kernel/init.rs            | 22 +++++++++++-----------
 rust/kernel/init/__internal.rs |  4 ++--
 rust/kernel/init/macros.rs     | 10 +++++-----
 rust/kernel/ioctl.rs           |  2 +-
 rust/kernel/lib.rs             |  1 +
 rust/kernel/list/arc_field.rs  |  2 +-
 rust/kernel/print.rs           |  4 ++--
 rust/kernel/std_vendor.rs      | 10 +++++-----
 samples/rust/rust_print.rs     |  2 +-
 scripts/Makefile.build         |  2 +-
 11 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 639bc7572f90..a681acda87ce 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -133,7 +133,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
     }
 
     /// Returns the error encoded as a pointer.
-    #[allow(dead_code)]
+    #[expect(dead_code)]
     pub(crate) fn to_ptr<T>(self) -> *mut T {
         #[cfg_attr(target_pointer_width = "32", allow(clippy::useless_conversion))]
         // SAFETY: `self.0` is a valid error due to its invariant.
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 10ec90a5f5d8..25057cbed40b 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -35,7 +35,7 @@
 //! that you need to write `<-` instead of `:` for fields that you want to initialize in-place.
 //!
 //! ```rust
-//! # #![allow(clippy::disallowed_names)]
+//! # #![expect(clippy::disallowed_names)]
 //! use kernel::sync::{new_mutex, Mutex};
 //! # use core::pin::Pin;
 //! #[pin_data]
@@ -55,7 +55,7 @@
 //! (or just the stack) to actually initialize a `Foo`:
 //!
 //! ```rust
-//! # #![allow(clippy::disallowed_names)]
+//! # #![expect(clippy::disallowed_names)]
 //! # use kernel::sync::{new_mutex, Mutex};
 //! # use core::pin::Pin;
 //! # #[pin_data]
@@ -120,12 +120,12 @@
 //!   `slot` gets called.
 //!
 //! ```rust
-//! # #![allow(unreachable_pub, clippy::disallowed_names)]
+//! # #![expect(unreachable_pub, clippy::disallowed_names)]
 //! use kernel::{init, types::Opaque};
 //! use core::{ptr::addr_of_mut, marker::PhantomPinned, pin::Pin};
 //! # mod bindings {
-//! #     #![allow(non_camel_case_types)]
-//! #     #![allow(clippy::missing_safety_doc)]
+//! #     #![expect(non_camel_case_types)]
+//! #     #![expect(clippy::missing_safety_doc)]
 //! #     pub struct foo;
 //! #     pub unsafe fn init_foo(_ptr: *mut foo) {}
 //! #     pub unsafe fn destroy_foo(_ptr: *mut foo) {}
@@ -238,7 +238,7 @@
 /// # Examples
 ///
 /// ```rust
-/// # #![allow(clippy::disallowed_names)]
+/// # #![expect(clippy::disallowed_names)]
 /// # use kernel::{init, macros::pin_data, pin_init, stack_pin_init, init::*, sync::Mutex, new_mutex};
 /// # use core::pin::Pin;
 /// #[pin_data]
@@ -290,7 +290,7 @@ macro_rules! stack_pin_init {
 /// # Examples
 ///
 /// ```rust,ignore
-/// # #![allow(clippy::disallowed_names)]
+/// # #![expect(clippy::disallowed_names)]
 /// # use kernel::{init, pin_init, stack_try_pin_init, init::*, sync::Mutex, new_mutex};
 /// # use macros::pin_data;
 /// # use core::{alloc::AllocError, pin::Pin};
@@ -316,7 +316,7 @@ macro_rules! stack_pin_init {
 /// ```
 ///
 /// ```rust,ignore
-/// # #![allow(clippy::disallowed_names)]
+/// # #![expect(clippy::disallowed_names)]
 /// # use kernel::{init, pin_init, stack_try_pin_init, init::*, sync::Mutex, new_mutex};
 /// # use macros::pin_data;
 /// # use core::{alloc::AllocError, pin::Pin};
@@ -438,7 +438,7 @@ macro_rules! stack_try_pin_init {
 /// Users of `Foo` can now create it like this:
 ///
 /// ```rust
-/// # #![allow(clippy::disallowed_names)]
+/// # #![expect(clippy::disallowed_names)]
 /// # use kernel::{init, pin_init, macros::pin_data, init::*};
 /// # use core::pin::Pin;
 /// # #[pin_data]
@@ -852,7 +852,7 @@ pub unsafe trait PinInit<T: ?Sized, E = Infallible>: Sized {
     /// # Examples
     ///
     /// ```rust
-    /// # #![allow(clippy::disallowed_names)]
+    /// # #![expect(clippy::disallowed_names)]
     /// use kernel::{types::Opaque, init::pin_init_from_closure};
     /// #[repr(C)]
     /// struct RawFoo([u8; 16]);
@@ -964,7 +964,7 @@ pub unsafe trait Init<T: ?Sized, E = Infallible>: PinInit<T, E> {
     /// # Examples
     ///
     /// ```rust
-    /// # #![allow(clippy::disallowed_names)]
+    /// # #![expect(clippy::disallowed_names)]
     /// use kernel::{types::Opaque, init::{self, init_from_closure}};
     /// struct Foo {
     ///     buf: [u8; 1_000_000],
diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
index 549ae227c2ea..44431fba7aab 100644
--- a/rust/kernel/init/__internal.rs
+++ b/rust/kernel/init/__internal.rs
@@ -54,7 +54,7 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
 pub unsafe trait HasPinData {
     type PinData: PinData;
 
-    #[allow(clippy::missing_safety_doc)]
+    #[expect(clippy::missing_safety_doc)]
     unsafe fn __pin_data() -> Self::PinData;
 }
 
@@ -84,7 +84,7 @@ fn make_closure<F, O, E>(self, f: F) -> F
 pub unsafe trait HasInitData {
     type InitData: InitData;
 
-    #[allow(clippy::missing_safety_doc)]
+    #[expect(clippy::missing_safety_doc)]
     unsafe fn __init_data() -> Self::InitData;
 }
 
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 8733ba3834ab..dcc280c7581f 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -182,13 +182,13 @@
 //!     // Normally `Drop` bounds do not have the correct semantics, but for this purpose they do
 //!     // (normally people want to know if a type has any kind of drop glue at all, here we want
 //!     // to know if it has any kind of custom drop glue, which is exactly what this bound does).
-//!     #[allow(drop_bounds)]
+//!     #[expect(drop_bounds)]
 //!     impl<T: ::core::ops::Drop> MustNotImplDrop for T {}
 //!     impl<T> MustNotImplDrop for Bar<T> {}
 //!     // Here comes a convenience check, if one implemented `PinnedDrop`, but forgot to add it to
 //!     // `#[pin_data]`, then this will error with the same mechanic as above, this is not needed
 //!     // for safety, but a good sanity check, since no normal code calls `PinnedDrop::drop`.
-//!     #[allow(non_camel_case_types)]
+//!     #[expect(non_camel_case_types)]
 //!     trait UselessPinnedDropImpl_you_need_to_specify_PinnedDrop {}
 //!     impl<
 //!         T: ::kernel::init::PinnedDrop,
@@ -925,14 +925,14 @@ impl<'__pin, $($impl_generics)*> ::core::marker::Unpin for $name<$($ty_generics)
         // `Drop`. Additionally we will implement this trait for the struct leading to a conflict,
         // if it also implements `Drop`
         trait MustNotImplDrop {}
-        #[allow(drop_bounds)]
+        #[expect(drop_bounds)]
         impl<T: ::core::ops::Drop> MustNotImplDrop for T {}
         impl<$($impl_generics)*> MustNotImplDrop for $name<$($ty_generics)*>
         where $($whr)* {}
         // We also take care to prevent users from writing a useless `PinnedDrop` implementation.
         // They might implement `PinnedDrop` correctly for the struct, but forget to give
         // `PinnedDrop` as the parameter to `#[pin_data]`.
-        #[allow(non_camel_case_types)]
+        #[expect(non_camel_case_types)]
         trait UselessPinnedDropImpl_you_need_to_specify_PinnedDrop {}
         impl<T: $crate::init::PinnedDrop>
             UselessPinnedDropImpl_you_need_to_specify_PinnedDrop for T {}
@@ -989,7 +989,7 @@ fn drop(&mut self) {
         //
         // The functions are `unsafe` to prevent accidentally calling them.
         #[allow(dead_code)]
-        #[allow(clippy::missing_safety_doc)]
+        #[expect(clippy::missing_safety_doc)]
         impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
         where $($whr)*
         {
diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs
index cfa7d080b531..2fc7662339e5 100644
--- a/rust/kernel/ioctl.rs
+++ b/rust/kernel/ioctl.rs
@@ -4,7 +4,7 @@
 //!
 //! C header: [`include/asm-generic/ioctl.h`](srctree/include/asm-generic/ioctl.h)
 
-#![allow(non_snake_case)]
+#![expect(non_snake_case)]
 
 use crate::build_assert;
 
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f10b06a78b9d..7d1c97a21fe1 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -14,6 +14,7 @@
 #![no_std]
 #![feature(coerce_unsized)]
 #![feature(dispatch_from_dyn)]
+#![feature(lint_reasons)]
 #![feature(new_uninit)]
 #![feature(receiver_trait)]
 #![feature(unsize)]
diff --git a/rust/kernel/list/arc_field.rs b/rust/kernel/list/arc_field.rs
index 2330f673427a..c4b9dd503982 100644
--- a/rust/kernel/list/arc_field.rs
+++ b/rust/kernel/list/arc_field.rs
@@ -56,7 +56,7 @@ pub unsafe fn assert_ref(&self) -> &T {
     ///
     /// The caller must have mutable access to the `ListArc<ID>` containing the struct with this
     /// field for the duration of the returned reference.
-    #[allow(clippy::mut_from_ref)]
+    #[expect(clippy::mut_from_ref)]
     pub unsafe fn assert_mut(&self) -> &mut T {
         // SAFETY: The caller has exclusive access to the `ListArc`, so they also have exclusive
         // access to this field.
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 45af17095a24..a28077a7cb30 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -14,7 +14,7 @@
 use crate::str::RawFormatter;
 
 // Called from `vsprintf` with format specifier `%pA`.
-#[allow(clippy::missing_safety_doc)]
+#[expect(clippy::missing_safety_doc)]
 #[no_mangle]
 unsafe extern "C" fn rust_fmt_argument(
     buf: *mut c_char,
@@ -140,7 +140,7 @@ pub fn call_printk_cont(args: fmt::Arguments<'_>) {
 #[doc(hidden)]
 #[cfg(not(testlib))]
 #[macro_export]
-#[allow(clippy::crate_in_macro_def)]
+#[expect(clippy::crate_in_macro_def)]
 macro_rules! print_macro (
     // The non-continuation cases (most of them, e.g. `INFO`).
     ($format_string:path, false, $($arg:tt)+) => (
diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
index d59e4cf4b252..8b4872b48e97 100644
--- a/rust/kernel/std_vendor.rs
+++ b/rust/kernel/std_vendor.rs
@@ -16,7 +16,7 @@
 ///
 /// ```rust
 /// let a = 2;
-/// # #[allow(clippy::disallowed_macros)]
+/// # #[expect(clippy::disallowed_macros)]
 /// let b = dbg!(a * 2) + 1;
 /// //      ^-- prints: [src/main.rs:2] a * 2 = 4
 /// assert_eq!(b, 5);
@@ -54,7 +54,7 @@
 /// With a method call:
 ///
 /// ```rust
-/// # #[allow(clippy::disallowed_macros)]
+/// # #[expect(clippy::disallowed_macros)]
 /// fn foo(n: usize) {
 ///     if dbg!(n.checked_sub(4)).is_some() {
 ///         // ...
@@ -73,7 +73,7 @@
 /// Naive factorial implementation:
 ///
 /// ```rust
-/// # #[allow(clippy::disallowed_macros)]
+/// # #[expect(clippy::disallowed_macros)]
 /// # {
 /// fn factorial(n: u32) -> u32 {
 ///     if dbg!(n <= 1) {
@@ -120,7 +120,7 @@
 /// a tuple (and return it, too):
 ///
 /// ```
-/// # #![allow(clippy::disallowed_macros)]
+/// # #![expect(clippy::disallowed_macros)]
 /// assert_eq!(dbg!(1usize, 2u32), (1, 2));
 /// ```
 ///
@@ -129,7 +129,7 @@
 /// invocations. You can use a 1-tuple directly if you need one:
 ///
 /// ```
-/// # #[allow(clippy::disallowed_macros)]
+/// # #[expect(clippy::disallowed_macros)]
 /// # {
 /// assert_eq!(1, dbg!(1u32,)); // trailing comma ignored
 /// assert_eq!((1,), dbg!((1u32,))); // 1-tuple
diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs
index ed1137ab2018..ba1606bdbd75 100644
--- a/samples/rust/rust_print.rs
+++ b/samples/rust/rust_print.rs
@@ -15,7 +15,7 @@
 
 struct RustPrint;
 
-#[allow(clippy::disallowed_macros)]
+#[expect(clippy::disallowed_macros)]
 fn arc_print() -> Result {
     use kernel::sync::*;
 
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 72b1232b1f7d..0910f6390c93 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -263,7 +263,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE
 # Compile Rust sources (.rs)
 # ---------------------------------------------------------------------------
 
-rust_allowed_features := new_uninit
+rust_allowed_features := lint_reasons,new_uninit
 
 # `--out-dir` is required to avoid temporaries being created by `rustc` in the
 # current working directory, which may be not accessible in the out-of-tree
-- 
2.46.0

Re: [PATCH 17/19] rust: start using the `#[expect(...)]` attribute

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust, it is possible to `allow` particular warnings (diagnostics,
> lints) locally, making the compiler ignore instances of a given warning
> within a given function, module, block, etc.
>
> It is similar to `#pragma GCC diagnostic push` + `ignored` + `pop` in C:
>
>     #pragma GCC diagnostic push
>     #pragma GCC diagnostic ignored "-Wunused-function"
>     static void f(void) {}
>     #pragma GCC diagnostic pop
>
> But way less verbose:
>
>     #[allow(dead_code)]
>     fn f() {}
>
> By that virtue, it makes it possible to comfortably enable more
> diagnostics by default (i.e. outside `W=` levels) that may have some
> false positives but that are otherwise quite useful to keep enabled to
> catch potential mistakes.
>
> The `#[expect(...)]` attribute [1] takes this further, and makes the
> compiler warn if the diagnostic was _not_ produced. For instance, the
> following will ensure that, when `f()` is called somewhere, we will have
> to remove the attribute:
>
>     #[expect(dead_code)]
>     fn f() {}
>
> If we do not, we get a warning from the compiler:
>
>     warning: this lint expectation is unfulfilled
>      --> x.rs:3:10
>       |
>     3 | #[expect(dead_code)]
>       |          ^^^^^^^^^
>       |
>       = note: `#[warn(unfulfilled_lint_expectations)]` on by default
>
> This means that `expect`s do not get forgotten when they are not needed.
>
> See the next commit for more details, nuances on its usage and
> documentation on the feature.
>
> The attribute requires the `lint_reasons` [2] unstable feature, but it
> is becoming stable in 1.81.0 (to be released on 2024-09-05) and it has
> already been useful to clean things up in this patch series, finding
> cases where the `allow`s should not have been there.
>
> Thus, enable `lint_reasons` and convert some of our `allow`s to `expect`s
> where possible.
>
> This feature was also an example of the ongoing collaboration between
> Rust and the kernel -- we tested it in the kernel early on and found an
> issue that was quickly resolved [3].
>
> Cc: Fridtjof Stoldt <xfrednet@gmail.com>
> Cc: Urgau <urgau@numericable.fr>
> Link: https://rust-lang.github.io/rfcs/2383-lint-reasons.html#expect-lint-attribute [1]
> Link: https://github.com/rust-lang/rust/issues/54503 [2]
> Link: https://github.com/rust-lang/rust/issues/114557 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 17/19] rust: start using the `#[expect(...)]` attribute

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In Rust, it is possible to `allow` particular warnings (diagnostics,
> lints) locally, making the compiler ignore instances of a given warning
> within a given function, module, block, etc.
>
> It is similar to `#pragma GCC diagnostic push` + `ignored` + `pop` in C:
>
>     #pragma GCC diagnostic push
>     #pragma GCC diagnostic ignored "-Wunused-function"
>     static void f(void) {}
>     #pragma GCC diagnostic pop
>
> But way less verbose:
>
>     #[allow(dead_code)]
>     fn f() {}
>
> By that virtue, it makes it possible to comfortably enable more
> diagnostics by default (i.e. outside `W=` levels) that may have some
> false positives but that are otherwise quite useful to keep enabled to
> catch potential mistakes.
>
> The `#[expect(...)]` attribute [1] takes this further, and makes the
> compiler warn if the diagnostic was _not_ produced. For instance, the
> following will ensure that, when `f()` is called somewhere, we will have
> to remove the attribute:
>
>     #[expect(dead_code)]
>     fn f() {}
>
> If we do not, we get a warning from the compiler:
>
>     warning: this lint expectation is unfulfilled
>      --> x.rs:3:10
>       |
>     3 | #[expect(dead_code)]
>       |          ^^^^^^^^^
>       |
>       = note: `#[warn(unfulfilled_lint_expectations)]` on by default
>
> This means that `expect`s do not get forgotten when they are not needed.
>
> See the next commit for more details, nuances on its usage and
> documentation on the feature.
>
> The attribute requires the `lint_reasons` [2] unstable feature, but it
> is becoming stable in 1.81.0 (to be released on 2024-09-05) and it has
> already been useful to clean things up in this patch series, finding
> cases where the `allow`s should not have been there.
>
> Thus, enable `lint_reasons` and convert some of our `allow`s to `expect`s
> where possible.
>
> This feature was also an example of the ongoing collaboration between
> Rust and the kernel -- we tested it in the kernel early on and found an
> issue that was quickly resolved [3].
>
> Cc: Fridtjof Stoldt <xfrednet@gmail.com>
> Cc: Urgau <urgau@numericable.fr>
> Link: https://rust-lang.github.io/rfcs/2383-lint-reasons.html#expect-lint-attribute [1]
> Link: https://github.com/rust-lang/rust/issues/54503 [2]
> Link: https://github.com/rust-lang/rust/issues/114557 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH 18/19] Documentation: rust: discuss `#[expect(...)]` in the guidelines

[Miguel Ojeda]

Discuss `#[expect(...)]` in the Lints sections of the coding guidelines
document, which is an upcoming feature in Rust 1.81.0, and explain that
it is generally to be preferred over `allow` unless there is a reason
not to use it (e.g. conditional compilation being involved).

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/rust/coding-guidelines.rst | 110 +++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst
index 185d3b51117d..5d64bc69acae 100644
--- a/Documentation/rust/coding-guidelines.rst
+++ b/Documentation/rust/coding-guidelines.rst
@@ -256,3 +256,113 @@ By that virtue, it makes it possible to comfortably enable more diagnostics by
 default (i.e. outside ``W=`` levels). In particular, those that may have some
 false positives but that are otherwise quite useful to keep enabled to catch
 potential mistakes.
+
+On top of that, Rust provides the ``expect`` attribute which takes this further.
+It makes the compiler warn if the warning was not produced. For instance, the
+following will ensure that, when ``f()`` is called somewhere, we will have to
+remove the attribute:
+
+.. code-block:: rust
+
+	#[expect(dead_code)]
+	fn f() {}
+
+If we do not, we get a warning from the compiler::
+
+	warning: this lint expectation is unfulfilled
+	 --> x.rs:3:10
+	  |
+	3 | #[expect(dead_code)]
+	  |          ^^^^^^^^^
+	  |
+	  = note: `#[warn(unfulfilled_lint_expectations)]` on by default
+
+This means that ``expect``\ s do not get forgotten when they are not needed, which
+may happen in several situations, e.g.:
+
+- Temporary attributes added while developing.
+
+- Improvements in lints in the compiler, Clippy or custom tools which may
+  remove a false positive.
+
+- When the lint is not needed anymore because it was expected that it would be
+  removed at some point, such as the ``dead_code`` example above.
+
+It also increases the visibility of the remaining ``allow``\ s and reduces the
+chance of misapplying one.
+
+Thus prefer ``except`` over ``allow`` unless:
+
+- The lint attribute is intended to be temporary, e.g. while developing.
+
+- Conditional compilation triggers the warning in some cases but not others.
+
+  If there are only a few cases where the warning triggers (or does not
+  trigger) compared to the total number of cases, then one may consider using
+  a conditional ``expect`` (i.e. ``cfg_attr(..., expect(...))``). Otherwise,
+  it is likely simpler to just use ``allow``.
+
+- Inside macros, when the different invocations may create expanded code that
+  triggers the warning in some cases but not in others.
+
+- When code may trigger a warning for some architectures but not others, such
+  as an ``as`` cast to a C FFI type.
+
+As a more developed example, consider for instance this program:
+
+.. code-block:: rust
+
+	fn g() {}
+
+	fn main() {
+	    #[cfg(CONFIG_X)]
+	    g();
+	}
+
+Here, function ``g()`` is dead code if ``CONFIG_X`` is not set. Can we use
+``expect`` here?
+
+.. code-block:: rust
+
+	#[expect(dead_code)]
+	fn g() {}
+
+	fn main() {
+	    #[cfg(CONFIG_X)]
+	    g();
+	}
+
+This would emit a lint if ``CONFIG_X`` is set, since it is not dead code in that
+configuration. Therefore, in cases like this, we cannot use ``expect`` as-is.
+
+A simple possibility is using ``allow``:
+
+.. code-block:: rust
+
+	#[allow(dead_code)]
+	fn g() {}
+
+	fn main() {
+	    #[cfg(CONFIG_X)]
+	    g();
+	}
+
+An alternative would be using a conditional ``expect``:
+
+.. code-block:: rust
+
+	#[cfg_attr(not(CONFIG_X), expect(dead_code))]
+	fn g() {}
+
+	fn main() {
+	    #[cfg(CONFIG_X)]
+	    g();
+	}
+
+This would ensure that, if someone introduces another call to ``g()`` somewhere
+(e.g. unconditionally), then it would be spotted that it is not dead code
+anymore. However, the ``cfg_attr`` is more complex than a simple ``allow``.
+
+Therefore, it is likely that it is not worth using conditional ``expect``\ s when
+more than one or two configurations are involved or when the lint may be
+triggered due to non-local changes (such as ``dead_code``).
-- 
2.46.0

Re: [PATCH 18/19] Documentation: rust: discuss `#[expect(...)]` in the guidelines

[Trevor Gross]

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Discuss `#[expect(...)]` in the Lints sections of the coding guidelines
> document, which is an upcoming feature in Rust 1.81.0, and explain that
> it is generally to be preferred over `allow` unless there is a reason
> not to use it (e.g. conditional compilation being involved).
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Would it be good to mention that a reason can be specified with
`reason = "..."`? I don't think we use this anywhere yet.

Re: [PATCH 18/19] Documentation: rust: discuss `#[expect(...)]` in the guidelines

[Miguel Ojeda]

On Sun, Sep 29, 2024 at 7:11 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> Would it be good to mention that a reason can be specified with
> `reason = "..."`? I don't think we use this anywhere yet.

I mainly wanted to introduce things "slowly" :)

But I would be happy if we consistently use `reason`, I think it is a
good idea to document those, and perhaps we can get to the point where
we go for `clippy::allow_attributes_without_reason` too.

Relatedly, I also thought about `clippy::allow_attributes`, but due to
conditional compilation and the other reasons mentioned in the docs
added in the series, it is likely not something we can easily do.

Another tangent: I find the `reason` syntax a bit too "busy". I think
in some cases we may just want to write things as if they were "normal
comments" (and multiline and so on). Highlighting in particular ways
could help perhaps. It would have been nice to have something like `//
ALLOW` on top of the attribute, similar to `// SAFETY`, and have the
compiler understand that directly like Clippy nowadays does for `//
SAFETY`, but I guess there are downsides.

Cheers,
Miguel

[PATCH 19/19] rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes

[Miguel Ojeda]

It is cleaner to have a single inner attribute rather than needing
several hidden lines to wrap the macro invocations.

Thus simplify them.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/std_vendor.rs | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
index 8b4872b48e97..8f27d447a7fc 100644
--- a/rust/kernel/std_vendor.rs
+++ b/rust/kernel/std_vendor.rs
@@ -73,8 +73,7 @@
 /// Naive factorial implementation:
 ///
 /// ```rust
-/// # #[expect(clippy::disallowed_macros)]
-/// # {
+/// # #![expect(clippy::disallowed_macros)]
 /// fn factorial(n: u32) -> u32 {
 ///     if dbg!(n <= 1) {
 ///         dbg!(1)
@@ -84,7 +83,6 @@
 /// }
 ///
 /// dbg!(factorial(4));
-/// # }
 /// ```
 ///
 /// This prints to the kernel log:
@@ -129,11 +127,9 @@
 /// invocations. You can use a 1-tuple directly if you need one:
 ///
 /// ```
-/// # #[expect(clippy::disallowed_macros)]
-/// # {
+/// # #![expect(clippy::disallowed_macros)]
 /// assert_eq!(1, dbg!(1u32,)); // trailing comma ignored
 /// assert_eq!((1,), dbg!((1u32,))); // 1-tuple
-/// # }
 /// ```
 ///
 /// [`std::dbg`]: https://doc.rust-lang.org/std/macro.dbg.html
-- 
2.46.0

Re: [PATCH 19/19] rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes

[Alice Ryhl]

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> It is cleaner to have a single inner attribute rather than needing
> several hidden lines to wrap the macro invocations.
>
> Thus simplify them.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Are we ok with changing std_vendor?

Alice

Re: [PATCH 19/19] rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes

[Miguel Ojeda]

On Thu, Sep 5, 2024 at 10:19 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Are we ok with changing std_vendor?

Yeah, to some degree, i.e. the code is already adapted slightly (it is
not intended to be a 1:1 copy). As long as we keep it more or less in
sync with upstream, it should be fine.

Even if we had to diverge, it should not be a big deal, but it would
be nice to be able to pick up improvements if any, e.g. there is `let
else` being used now upstream, which we could replicate (I will create
a "good first issue" for that).

(These allows/expects are not there in the original anyway, for that reason)

Cheers,
Miguel

Re: [PATCH 19/19] rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes

[Alice Ryhl]

On Thu, Sep 5, 2024 at 11:27 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Thu, Sep 5, 2024 at 10:19 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Are we ok with changing std_vendor?
>
> Yeah, to some degree, i.e. the code is already adapted slightly (it is
> not intended to be a 1:1 copy). As long as we keep it more or less in
> sync with upstream, it should be fine.
>
> Even if we had to diverge, it should not be a big deal, but it would
> be nice to be able to pick up improvements if any, e.g. there is `let
> else` being used now upstream, which we could replicate (I will create
> a "good first issue" for that).
>
> (These allows/expects are not there in the original anyway, for that reason)
>
> Cheers,
> Miguel

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 19/19] rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes

[Vincenzo Palazzo]

On Wed Sep 4, 2024 at 10:43 PM CEST, Miguel Ojeda wrote:
> It is cleaner to have a single inner attribute rather than needing
> several hidden lines to wrap the macro invocations.
>
> Thus simplify them.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

Re: [PATCH 00/19] rust: lint improvements

[Gary Guo]

On Wed,  4 Sep 2024 22:43:28 +0200
Miguel Ojeda <ojeda@kernel.org> wrote:

> Hi all,
> 
> This is a series that contains a series of lint-related things:
> 
>   - Cleanups and other improvements and fixes, including removing `allow`s that
>     are not needed anymore for different reasons and a workaround for
>     `dbg_macro` detection.
> 
>   - The enablement of some safety lints so that the toolchain enforces that we
>     write `// SAFETY` comments and `# Safety` sections properly.
> 
>   - The addition of `.clippy.toml`, which allows us to take advantage of a few
>     configuration options.
> 
>   - Start using the new `#[expect(...)]` feature and add documentation on it as
>     well as lints in general.
> 
> Overall, this should improve the quality of the code and documentation as well
> as reduce the time needed in reviews.
> 
> I want to mention Trevor's nice work on lints from a while ago [1]. I think we
> should still do something like that: discuss which lints we would like to have
> one-by-one and start enabling them (and perhaps have a file like Trevor proposed
> etc.).
> 
> For the moment, though, I am sending these, since we would like to have at least
> the safety-related ones enabled soon [2]: now that more code and developers
> are joining, it sounds like a good time to start enforcing it -- it should make
> new Rust kernel developers aware of the need of writing them, which has proven
> to be a common request from reviewers.
> 
> If needed, the series can be applied partially or split, but most of it should
> be fairly uncontroversial.
> 
> Link: https://github.com/Rust-for-Linux/linux/pull/1025 [1]
> Link: https://lore.kernel.org/rust-for-linux/CD29DF8F-7FF3-466F-9724-BC92C14A68BD@collabora.com/ [2]

Built with clippy locally and it's all clean.

Tested-by: Gary Guo <gary@garyguo.net>

Apart from one `#[]`/`#![]` modification - which I think is fine as
long as it's intentional, everything LGTM.

Reviewed-by: Gary Guo <gary@garyguo.net>


> 
> Miguel Ojeda (19):
>   rust: workqueue: remove unneeded ``#[allow(clippy::new_ret_no_self)]`
>   rust: sort global Rust flags
>   rust: types: avoid repetition in `{As,From}Bytes` impls
>   rust: enable `clippy::undocumented_unsafe_blocks` lint
>   rust: enable `clippy::unnecessary_safety_comment` lint
>   rust: enable `clippy::unnecessary_safety_doc` lint
>   rust: enable `clippy::ignored_unit_patterns` lint
>   rust: enable `rustdoc::unescaped_backticks` lint
>   rust: init: remove unneeded `#[allow(clippy::disallowed_names)]`
>   rust: sync: remove unneeded
>     `#[allow(clippy::non_send_fields_in_send_ty)]`
>   rust: introduce `.clippy.toml`
>   rust: replace `clippy::dbg_macro` with `disallowed_macros`
>   rust: rbtree: fix `SAFETY` comments that should be `# Safety` sections
>   rust: provide proper code documentation titles
>   rust: enable Clippy's `check-private-items`
>   Documentation: rust: add coding guidelines on lints
>   rust: start using the `#[expect(...)]` attribute
>   Documentation: rust: discuss `#[expect(...)]` in the guidelines
>   rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes
> 
>  .clippy.toml                             |   9 ++
>  .gitignore                               |   1 +
>  Documentation/rust/coding-guidelines.rst | 139 +++++++++++++++++++++++
>  MAINTAINERS                              |   1 +
>  Makefile                                 |  15 ++-
>  rust/Makefile                            |   5 +-
>  rust/bindings/lib.rs                     |   1 +
>  rust/kernel/alloc/allocator.rs           |   2 +
>  rust/kernel/error.rs                     |  11 +-
>  rust/kernel/init.rs                      |  30 ++---
>  rust/kernel/init/__internal.rs           |  11 +-
>  rust/kernel/init/macros.rs               |  18 ++-
>  rust/kernel/ioctl.rs                     |   2 +-
>  rust/kernel/lib.rs                       |   1 +
>  rust/kernel/list.rs                      |   1 +
>  rust/kernel/list/arc_field.rs            |   2 +-
>  rust/kernel/print.rs                     |   5 +-
>  rust/kernel/rbtree.rs                    |   9 +-
>  rust/kernel/std_vendor.rs                |  16 ++-
>  rust/kernel/str.rs                       |   7 +-
>  rust/kernel/sync/arc.rs                  |   2 +-
>  rust/kernel/sync/arc/std_vendor.rs       |   2 +
>  rust/kernel/sync/condvar.rs              |   1 -
>  rust/kernel/sync/lock.rs                 |   6 +-
>  rust/kernel/types.rs                     |  74 ++++++------
>  rust/kernel/workqueue.rs                 |   9 +-
>  rust/uapi/lib.rs                         |   1 +
>  samples/rust/rust_print.rs               |   1 +
>  scripts/Makefile.build                   |   2 +-
>  29 files changed, 293 insertions(+), 91 deletions(-)
>  create mode 100644 .clippy.toml
> 
> 
> base-commit: 68d3b6aa08708bb3907c2c13eaf4b3ccf4805160
> --
> 2.46.0

Re: [PATCH 00/19] rust: lint improvements

[Miguel Ojeda]

On Wed, Sep 4, 2024 at 10:44 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Hi all,
>
> This is a series that contains a series of lint-related things:
>
>   - Cleanups and other improvements and fixes, including removing `allow`s that
>     are not needed anymore for different reasons and a workaround for
>     `dbg_macro` detection.
>
>   - The enablement of some safety lints so that the toolchain enforces that we
>     write `// SAFETY` comments and `# Safety` sections properly.
>
>   - The addition of `.clippy.toml`, which allows us to take advantage of a few
>     configuration options.
>
>   - Start using the new `#[expect(...)]` feature and add documentation on it as
>     well as lints in general.
>
> Overall, this should improve the quality of the code and documentation as well
> as reduce the time needed in reviews.
>
> I want to mention Trevor's nice work on lints from a while ago [1]. I think we
> should still do something like that: discuss which lints we would like to have
> one-by-one and start enabling them (and perhaps have a file like Trevor proposed
> etc.).
>
> For the moment, though, I am sending these, since we would like to have at least
> the safety-related ones enabled soon [2]: now that more code and developers
> are joining, it sounds like a good time to start enforcing it -- it should make
> new Rust kernel developers aware of the need of writing them, which has proven
> to be a common request from reviewers.
>
> If needed, the series can be applied partially or split, but most of it should
> be fairly uncontroversial.
>
> Link: https://github.com/Rust-for-Linux/linux/pull/1025 [1]
> Link: https://lore.kernel.org/rust-for-linux/CD29DF8F-7FF3-466F-9724-BC92C14A68BD@collabora.com/ [2]

Applied to `rust-next` -- thanks everyone!

Cheers,
Miguel