Skip to content

Commit

Permalink
rust: start using the #[expect(...)] attribute
Browse files Browse the repository at this point in the history
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 <[email protected]>
Cc: Urgau <[email protected]>
Link: https://rust-lang.github.io/rfcs/2383-lint-reasons.html#expect-lint-attribute [1]
Link: rust-lang/rust#54503 [2]
Link: rust-lang/rust#114557 [3]
Reviewed-by: Alice Ryhl <[email protected]>
Reviewed-by: Trevor Gross <[email protected]>
Tested-by: Gary Guo <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
  • Loading branch information
ojeda committed Oct 3, 2024
1 parent 6e0d060 commit aa3501d
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 30 deletions.
2 changes: 1 addition & 1 deletion rust/kernel/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl Error {
}

/// 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.
Expand Down
22 changes: 11 additions & 11 deletions rust/kernel/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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) {}
Expand Down Expand Up @@ -238,7 +238,7 @@ pub mod macros;
/// # 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]
Expand Down Expand Up @@ -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};
Expand All @@ -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};
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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],
Expand Down
4 changes: 2 additions & 2 deletions rust/kernel/init/__internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ where
pub unsafe trait HasPinData {
type PinData: PinData;

#[allow(clippy::missing_safety_doc)]
#[expect(clippy::missing_safety_doc)]
unsafe fn __pin_data() -> Self::PinData;
}

Expand Down Expand Up @@ -84,7 +84,7 @@ pub unsafe trait PinData: Copy {
pub unsafe trait HasInitData {
type InitData: InitData;

#[allow(clippy::missing_safety_doc)]
#[expect(clippy::missing_safety_doc)]
unsafe fn __init_data() -> Self::InitData;
}

Expand Down
10 changes: 5 additions & 5 deletions rust/kernel/init/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -925,14 +925,14 @@ macro_rules! __pin_data {
// `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 {}
Expand Down Expand Up @@ -989,7 +989,7 @@ macro_rules! __pin_data {
//
// 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)*
{
Expand Down
2 changes: 1 addition & 1 deletion rust/kernel/ioctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions rust/kernel/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#![no_std]
#![feature(coerce_unsized)]
#![feature(dispatch_from_dyn)]
#![feature(lint_reasons)]
#![feature(new_uninit)]
#![feature(receiver_trait)]
#![feature(unsize)]
Expand Down
2 changes: 1 addition & 1 deletion rust/kernel/list/arc_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<T, const ID: u64> ListArcField<T, ID> {
///
/// 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.
Expand Down
4 changes: 2 additions & 2 deletions rust/kernel/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use core::{
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,
Expand Down Expand Up @@ -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)+) => (
Expand Down
10 changes: 5 additions & 5 deletions rust/kernel/std_vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
/// // ...
Expand All @@ -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) {
Expand Down Expand Up @@ -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));
/// ```
///
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion samples/rust/rust_print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module! {

struct RustPrint;

#[allow(clippy::disallowed_macros)]
#[expect(clippy::disallowed_macros)]
fn arc_print() -> Result {
use kernel::sync::*;

Expand Down
2 changes: 1 addition & 1 deletion scripts/Makefile.build
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,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
Expand Down

0 comments on commit aa3501d

Please sign in to comment.