From 99c7c4b0af6c1cee4ff845c2a4947f5d5ad0a4e3 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Wed, 4 Sep 2024 22:43:45 +0200 Subject: [PATCH] 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 Cc: Urgau 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: Alice Ryhl Reviewed-by: Trevor Gross Tested-by: Gary Guo Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240904204347.168520-18-ojeda@kernel.org Signed-off-by: Miguel Ojeda (cherry picked from commit 1f9ed172545687e5c04c77490a45896be6d2e459) [pinned_init: sync ui tests with new expanded code. - Paolo] Signed-off-by: Paolo Bonzini --- src/__internal.rs | 4 ++-- src/lib.rs | 14 +++++++------- src/macros.rs | 10 +++++----- tests/ui/expand/many_generics.expanded.rs | 1 + tests/ui/expand/pin-data.expanded.rs | 5 +++-- tests/ui/expand/pinned_drop.expanded.rs | 1 + 6 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/__internal.rs b/src/__internal.rs index 53e5b26..c6d8a95 100644 --- a/src/__internal.rs +++ b/src/__internal.rs @@ -62,7 +62,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; } @@ -92,7 +92,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; } diff --git a/src/lib.rs b/src/lib.rs index ce03ede..56e956f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,7 +62,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)] //! # #![feature(allocator_api)] //! use pinned_init::*; //! # use core::pin::Pin; @@ -85,7 +85,7 @@ //! (or just the stack) to actually initialize a `Foo`: //! //! ```rust -//! # #![allow(clippy::disallowed_names)] +//! # #![expect(clippy::disallowed_names)] //! # #![feature(allocator_api)] //! # #[path = "../examples/mutex.rs"] mod mutex; use mutex::*; //! # use pinned_init::*; @@ -277,7 +277,7 @@ pub use pinned_init_macro::{pin_data, pinned_drop, Zeroable}; /// # Examples /// /// ```rust -/// # #![allow(clippy::disallowed_names)] +/// # #![expect(clippy::disallowed_names)] /// # #![feature(allocator_api)] /// # #[path = "../examples/mutex.rs"] mod mutex; use mutex::*; /// # use pinned_init::*; @@ -329,7 +329,7 @@ macro_rules! stack_pin_init { /// # Examples /// /// ```rust -/// # #![allow(clippy::disallowed_names)] +/// # #![expect(clippy::disallowed_names)] /// # #![feature(allocator_api)] /// # #[path = "../examples/error.rs"] mod error; use error::Error; /// # #[path = "../examples/mutex.rs"] mod mutex; use mutex::*; @@ -356,7 +356,7 @@ macro_rules! stack_pin_init { /// ``` /// /// ```rust -/// # #![allow(clippy::disallowed_names)] +/// # #![expect(clippy::disallowed_names)] /// # #![feature(allocator_api)] /// # #[path = "../examples/error.rs"] mod error; use error::Error; /// # #[path = "../examples/mutex.rs"] mod mutex; use mutex::*; @@ -479,7 +479,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 pinned_init::*; /// # use core::pin::Pin; /// # #[pin_data] @@ -953,7 +953,7 @@ pub unsafe trait Init: PinInit { /// # Examples /// /// ```rust - /// # #![allow(clippy::disallowed_names)] + /// # #![expect(clippy::disallowed_names)] /// # use pinned_init::*; /// struct Foo { /// buf: [u8; 1_000_000], diff --git a/src/macros.rs b/src/macros.rs index b1f4a28..b865a4d 100644 --- a/src/macros.rs +++ b/src/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 MustNotImplDrop for T {} //! impl MustNotImplDrop for Bar {} //! // 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: ::pinned_init::PinnedDrop, @@ -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 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 UselessPinnedDropImpl_you_need_to_specify_PinnedDrop for T {} @@ -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)* { diff --git a/tests/ui/expand/many_generics.expanded.rs b/tests/ui/expand/many_generics.expanded.rs index 3f2f92e..b65af62 100644 --- a/tests/ui/expand/many_generics.expanded.rs +++ b/tests/ui/expand/many_generics.expanded.rs @@ -35,6 +35,7 @@ const _: () = { T: Bar<'a, 1>, {} #[allow(dead_code)] + #[expect(clippy::missing_safety_doc)] impl< 'a, 'b: 'a, diff --git a/tests/ui/expand/pin-data.expanded.rs b/tests/ui/expand/pin-data.expanded.rs index c75cceb..41291f5 100644 --- a/tests/ui/expand/pin-data.expanded.rs +++ b/tests/ui/expand/pin-data.expanded.rs @@ -15,6 +15,7 @@ const _: () = { } impl ::core::marker::Copy for __ThePinData {} #[allow(dead_code)] + #[expect(clippy::missing_safety_doc)] impl __ThePinData { unsafe fn _pin( self, @@ -54,10 +55,10 @@ const _: () = { __Unpin<'__pin>: ::core::marker::Unpin, {} trait MustNotImplDrop {} - #[allow(drop_bounds)] + #[expect(drop_bounds)] impl MustNotImplDrop for T {} impl MustNotImplDrop for Foo {} - #[allow(non_camel_case_types)] + #[expect(non_camel_case_types)] trait UselessPinnedDropImpl_you_need_to_specify_PinnedDrop {} impl< T: ::pinned_init::PinnedDrop, diff --git a/tests/ui/expand/pinned_drop.expanded.rs b/tests/ui/expand/pinned_drop.expanded.rs index 174ca88..d6a4757 100644 --- a/tests/ui/expand/pinned_drop.expanded.rs +++ b/tests/ui/expand/pinned_drop.expanded.rs @@ -15,6 +15,7 @@ const _: () = { } impl ::core::marker::Copy for __ThePinData {} #[allow(dead_code)] + #[expect(clippy::missing_safety_doc)] impl __ThePinData { unsafe fn _pin( self,