From 8577c9dca799bd74377f7c30015d8cdc53a53ca2 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Wed, 4 Sep 2024 22:43:40 +0200 Subject: rust: replace `clippy::dbg_macro` with `disallowed_macros` 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] Tested-by: Gary Guo Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240904204347.168520-13-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/std_vendor.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'rust/kernel/std_vendor.rs') 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 -- cgit v1.2.3 From 2f390cc589433dfcfedc307a141e103929a6fd4d Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Wed, 4 Sep 2024 22:43:42 +0200 Subject: rust: provide proper code documentation titles 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 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 repository, licensed under 5 | | //! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details, 6 | | //! see . | |_ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph Thus clean those two instances. 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] Reviewed-by: Trevor Gross Reviewed-by: Alice Ryhl Tested-by: Gary Guo Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240904204347.168520-15-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/std_vendor.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'rust/kernel/std_vendor.rs') 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 repository, licensed under //! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details, -- cgit v1.2.3 From 1f9ed172545687e5c04c77490a45896be6d2e459 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Wed, 4 Sep 2024 22:43:45 +0200 Subject: 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 --- rust/kernel/std_vendor.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'rust/kernel/std_vendor.rs') 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 -- cgit v1.2.3 From 1c71ddb310ad04ccec70441c7f17089514e3bab8 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Wed, 4 Sep 2024 22:43:47 +0200 Subject: rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes It is cleaner to have a single inner attribute rather than needing several hidden lines to wrap the macro invocations. Thus simplify them. Reviewed-by: Vincenzo Palazzo Reviewed-by: Alice Ryhl Tested-by: Gary Guo Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240904204347.168520-20-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/std_vendor.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'rust/kernel/std_vendor.rs') 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 -- cgit v1.2.3 From 3566362dd4dc61f770e42045a92cc3f17df61df0 Mon Sep 17 00:00:00 2001 From: Deepak Thukral Date: Fri, 4 Oct 2024 14:56:16 +0200 Subject: rust: std_vendor: update dbg macro from Rust upstream `dbg!` contains adapted code from Rust upstream. Compare the kernel code with the Rust upstream one and update missing column numbers in `dbg!` outputs. Column numbers are not copied but adjusted for the kernel's examples. Suggested-by: Miguel Ojeda Link: https://github.com/Rust-for-Linux/linux/issues/1124 Signed-off-by: Deepak Thukral Link: https://lore.kernel.org/r/20241004125616.49886-1-iapain@gmail.com [ Fixed typo and slightly reworded. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/std_vendor.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'rust/kernel/std_vendor.rs') diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs index 8f27d447a7fc..279bd353687a 100644 --- a/rust/kernel/std_vendor.rs +++ b/rust/kernel/std_vendor.rs @@ -18,7 +18,7 @@ /// let a = 2; /// # #[expect(clippy::disallowed_macros)] /// let b = dbg!(a * 2) + 1; -/// // ^-- prints: [src/main.rs:2] a * 2 = 4 +/// // ^-- prints: [src/main.rs:3:9] a * 2 = 4 /// assert_eq!(b, 5); /// ``` /// @@ -67,7 +67,7 @@ /// This prints to the kernel log: /// /// ```text,ignore -/// [src/main.rs:4] n.checked_sub(4) = None +/// [src/main.rs:3:8] n.checked_sub(4) = None /// ``` /// /// Naive factorial implementation: @@ -88,15 +88,15 @@ /// This prints to the kernel log: /// /// ```text,ignore -/// [src/main.rs:3] n <= 1 = false -/// [src/main.rs:3] n <= 1 = false -/// [src/main.rs:3] n <= 1 = false -/// [src/main.rs:3] n <= 1 = true -/// [src/main.rs:4] 1 = 1 -/// [src/main.rs:5] n * factorial(n - 1) = 2 -/// [src/main.rs:5] n * factorial(n - 1) = 6 -/// [src/main.rs:5] n * factorial(n - 1) = 24 -/// [src/main.rs:11] factorial(4) = 24 +/// [src/main.rs:3:8] n <= 1 = false +/// [src/main.rs:3:8] n <= 1 = false +/// [src/main.rs:3:8] n <= 1 = false +/// [src/main.rs:3:8] n <= 1 = true +/// [src/main.rs:4:9] 1 = 1 +/// [src/main.rs:5:9] n * factorial(n - 1) = 2 +/// [src/main.rs:5:9] n * factorial(n - 1) = 6 +/// [src/main.rs:5:9] n * factorial(n - 1) = 24 +/// [src/main.rs:11:1] factorial(4) = 24 /// ``` /// /// The `dbg!(..)` macro moves the input: -- cgit v1.2.3