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 <xfrednet@gmail.com>
Cc: Urgau <urgau@numericable.fr>
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 <aliceryhl@google.com>
Reviewed-by: Trevor Gross <tmgross@umich.edu>
Tested-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/r/20240904204347.168520-18-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
(cherry picked from commit 1f9ed172545687e5c04c77490a45896be6d2e459)
[pinned_init: sync ui tests with new expanded code. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
ojeda authored and bonzini committed Dec 2, 2024
1 parent 06d2aca commit 99c7c4b
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/__internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
14 changes: 7 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::*;
Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -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::*;
Expand All @@ -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::*;
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -953,7 +953,7 @@ pub unsafe trait Init<T: ?Sized, E = Infallible>: PinInit<T, E> {
/// # Examples
///
/// ```rust
/// # #![allow(clippy::disallowed_names)]
/// # #![expect(clippy::disallowed_names)]
/// # use pinned_init::*;
/// struct Foo {
/// buf: [u8; 1_000_000],
Expand Down
10 changes: 5 additions & 5 deletions src/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: ::pinned_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::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
1 change: 1 addition & 0 deletions tests/ui/expand/many_generics.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const _: () = {
T: Bar<'a, 1>,
{}
#[allow(dead_code)]
#[expect(clippy::missing_safety_doc)]
impl<
'a,
'b: 'a,
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/expand/pin-data.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const _: () = {
}
impl ::core::marker::Copy for __ThePinData {}
#[allow(dead_code)]
#[expect(clippy::missing_safety_doc)]
impl __ThePinData {
unsafe fn _pin<E>(
self,
Expand Down Expand Up @@ -54,10 +55,10 @@ const _: () = {
__Unpin<'__pin>: ::core::marker::Unpin,
{}
trait MustNotImplDrop {}
#[allow(drop_bounds)]
#[expect(drop_bounds)]
impl<T: ::core::ops::Drop> 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,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/expand/pinned_drop.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const _: () = {
}
impl ::core::marker::Copy for __ThePinData {}
#[allow(dead_code)]
#[expect(clippy::missing_safety_doc)]
impl __ThePinData {
unsafe fn _pin<E>(
self,
Expand Down

0 comments on commit 99c7c4b

Please sign in to comment.