Commit 1f9ed172 authored by Miguel Ojeda's avatar Miguel Ojeda
Browse files

rust: start using the `#[expect(...)]` attribute 

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]
Reviewed-by: default avatarAlice Ryhl <aliceryhl@google.com>
Reviewed-by: default avatarTrevor Gross <tmgross@umich.edu>
Tested-by: default avatarGary Guo <gary@garyguo.net>
Reviewed-by: default avatarGary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/r/20240904204347.168520-18-ojeda@kernel.org


Signed-off-by: default avatarMiguel Ojeda <ojeda@kernel.org>
parent 139d3965

rust/kernel/error.rs

+1 −1
Original line number Diff line number Diff line
@@ -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.

rust/kernel/init.rs

+11 −11
Original line number Diff line number Diff line
@@ -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],

rust/kernel/init/__internal.rs

+2 −2
Original line number Diff line number Diff line
@@ -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;

}

rust/kernel/init/macros.rs

+5 −5
Original line number Diff line number Diff line
@@ -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)*

        {

rust/kernel/ioctl.rs

+1 −1
Original line number Diff line number Diff line
@@ -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;