[PATCH v2 0/4] Enable rustdoc tests for the macros crate

[PATCH v2 0/4] Enable rustdoc tests for the macros crate

[Ethan D. Twardy]

Changes from v1:
 - Resolve a duplicate kbuild rule identified by the test robot
 - Fix a number of formatting issues in the tests

Ethan D. Twardy (4):
  kbuild: rust: Expand rusttest target for macros
  rust: Enable test for macros::module
  rust: macros: Enable use from macro_rules!
  rust: macros: Enable the rest of the tests

 rust/Makefile        |  29 ++++++++--
 rust/macros/lib.rs   | 126 ++++++++++++++++++++++++++++++++-----------
 rust/macros/paste.rs |  15 ++++--
 3 files changed, 132 insertions(+), 38 deletions(-)


base-commit: a126eca844353360ebafa9088d22865cb8e022e3
-- 
2.44.2

[PATCH v2 1/4] kbuild: rust: Expand rusttest target for macros

[Ethan D. Twardy]

Previously, the rusttest target for the macros crate did not specify
the dependencies necessary to run the rustdoc tests. These test rely on
the kernel crate, so add a specialized rustdoc tests command for this
particular crate.

Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

diff --git a/rust/Makefile b/rust/Makefile
index f70d5e244fee..df389df4db9c 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -147,6 +147,15 @@ rusttestlib-macros: private rustc_test_library_proc = yes
 rusttestlib-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
 	+$(call if_changed,rustc_test_library)
 
+rusttestlib-kernel: private rustc_target_flags = --extern alloc \
+    --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \
+    --extern bindings --extern uapi
+rusttestlib-kernel: $(src)/kernel/lib.rs rustdoc-compiler_builtins \
+    rustdoc-alloc rusttestlib-bindings rusttestlib-uapi rusttestlib-build_error \
+    $(obj)/libmacros.so \
+    $(obj)/bindings.o FORCE
+	+$(call if_changed,rustc_test_library)
+
 rusttestlib-bindings: $(src)/bindings/lib.rs rusttest-prepare FORCE
 	+$(call if_changed,rustc_test_library)
 
@@ -245,11 +254,24 @@ quiet_cmd_rustsysroot = RUSTSYSROOT
 rusttest-prepare: FORCE
 	+$(call if_changed,rustsysroot)
 
-rusttest-macros: private rustc_target_flags = --extern proc_macro
+quiet_cmd_rustdoc_test_macros = RUSTDOC T $<
+      cmd_rustdoc_test_macros = \
+	OBJTREE=$(abspath $(objtree)) \
+	$(RUSTDOC) --test $(rust_common_flags) \
+		@$(objtree)/include/generated/rustc_cfg \
+		$(rustc_target_flags) $(rustdoc_test_target_flags) \
+		--sysroot $(objtree)/$(obj)/test/sysroot $(rustdoc_test_quiet) \
+		-L$(objtree)/$(obj)/test --output $(rustdoc_output) \
+		-Zproc-macro-backtrace \
+		--crate-name $(subst rusttest-,,$@) $<
+
+rusttest-macros: private rustc_target_flags = --extern proc_macro \
+	--extern macros=$(objtree)/$(obj)/libmacros.so --extern kernel
 rusttest-macros: private rustdoc_test_target_flags = --crate-type proc-macro
-rusttest-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
+rusttest-macros: $(src)/macros/lib.rs rusttest-prepare \
+    rusttestlib-macros rusttestlib-kernel FORCE
 	+$(call if_changed,rustc_test)
-	+$(call if_changed,rustdoc_test)
+	+$(call if_changed,rustdoc_test_macros)
 
 rusttest-kernel: private rustc_target_flags = --extern alloc \
     --extern build_error --extern macros --extern bindings --extern uapi
-- 
2.44.2

[PATCH v2 2/4] rust: Enable test for macros::module

[Ethan D. Twardy]

Previously, this test was ignored due to a missing necessary dependency
on the `kernel` crate. Enable the test, and update it to remove the use
of a kernel parameter mechanism that was never merged.

Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

diff --git a/rust/Makefile b/rust/Makefile
index df389df4db9c..3ace8506ee8d 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -256,6 +256,7 @@ rusttest-prepare: FORCE
 
 quiet_cmd_rustdoc_test_macros = RUSTDOC T $<
       cmd_rustdoc_test_macros = \
+	export RUST_MODFILE=test.rs; \
 	OBJTREE=$(abspath $(objtree)) \
 	$(RUSTDOC) --test $(rust_common_flags) \
 		@$(objtree)/include/generated/rustc_cfg \
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 520eae5fd792..1c4ae5789cfa 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -26,9 +26,13 @@
 ///
 /// # Examples
 ///
-/// ```ignore
+/// ```rust
+/// # #[macro_use] extern crate macros;
+/// # #[macro_use] extern crate kernel;
 /// use kernel::prelude::*;
 ///
+/// struct MyModule(i32);
+///
 /// module!{
 ///     type: MyModule,
 ///     name: "my_kernel_module",
@@ -37,22 +41,14 @@
 ///     license: "GPL",
 /// }
 ///
-/// struct MyModule;
-///
 /// impl kernel::Module for MyModule {
-///     fn init() -> Result<Self> {
-///         // If the parameter is writeable, then the kparam lock must be
-///         // taken to read the parameter:
-///         {
-///             let lock = THIS_MODULE.kernel_param_lock();
-///             pr_info!("i32 param is:  {}\n", writeable_i32.read(&lock));
-///         }
-///         // If the parameter is read only, it can be read without locking
-///         // the kernel parameters:
-///         pr_info!("i32 param is:  {}\n", my_i32.read());
-///         Ok(Self)
+///     fn init(module: &'static ThisModule) -> Result<Self> {
+///         let foo: i32 = 42;
+///         pr_info!("I contain:  {}\n", foo);
+///         Ok(Self(foo))
 ///     }
 /// }
+/// # fn main() {}
 /// ```
 ///
 /// # Supported argument types
-- 
2.44.2

[PATCH v2 3/4] rust: macros: Enable use from macro_rules!

[Ethan D. Twardy]

According to the rustdoc for the proc_macro crate[1], tokens captured
from a "macro variable" (e.g. from within macro_rules!) may be delimited
by invisible tokens and be contained within a proc_macro::Group.
Previously, this scenario was not handled by macros::paste, which caused
a proc-macro panic when the corresponding tests are enabled. Enable the
tests, and handle this case by making macros::paste::concat recursive.

Link: https://doc.rust-lang.org/stable/proc_macro/enum.Delimiter.html [1]
Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 1c4ae5789cfa..3278f7c8aa5e 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -268,12 +268,25 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
 ///
 /// # Example
 ///
-/// ```ignore
-/// use kernel::macro::paste;
-///
+/// ```
+/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
+/// # const binder_driver_return_protocol_BR_ERROR: u32 = 1;
+/// # const binder_driver_return_protocol_BR_TRANSACTION: u32 = 2;
+/// # const binder_driver_return_protocol_BR_REPLY: u32 = 3;
+/// # const binder_driver_return_protocol_BR_DEAD_REPLY: u32 = 4;
+/// # const binder_driver_return_protocol_BR_TRANSACTION_COMPLETE: u32 = 5;
+/// # const binder_driver_return_protocol_BR_INCREFS: u32 = 6;
+/// # const binder_driver_return_protocol_BR_ACQUIRE: u32 = 7;
+/// # const binder_driver_return_protocol_BR_RELEASE: u32 = 8;
+/// # const binder_driver_return_protocol_BR_DECREFS: u32 = 9;
+/// # const binder_driver_return_protocol_BR_NOOP: u32 = 10;
+/// # const binder_driver_return_protocol_BR_SPAWN_LOOPER: u32 = 11;
+/// # const binder_driver_return_protocol_BR_DEAD_BINDER: u32 = 12;
+/// # const binder_driver_return_protocol_BR_CLEAR_DEATH_NOTIFICATION_DONE: u32 = 13;
+/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
 /// macro_rules! pub_no_prefix {
 ///     ($prefix:ident, $($newname:ident),+) => {
-///         paste! {
+///         kernel::macros::paste! {
 ///             $(pub(crate) const $newname: u32 = [<$prefix $newname>];)+
 ///         }
 ///     };
@@ -312,13 +325,28 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
 /// * `lower`: change the identifier to lower case.
 /// * `upper`: change the identifier to upper case.
 ///
-/// ```ignore
-/// use kernel::macro::paste;
-///
+/// ```rust
+/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
+/// # const binder_driver_return_protocol_BR_ERROR: u32 = 1;
+/// # const binder_driver_return_protocol_BR_TRANSACTION: u32 = 2;
+/// # const binder_driver_return_protocol_BR_REPLY: u32 = 3;
+/// # const binder_driver_return_protocol_BR_DEAD_REPLY: u32 = 4;
+/// # const binder_driver_return_protocol_BR_TRANSACTION_COMPLETE: u32 = 5;
+/// # const binder_driver_return_protocol_BR_INCREFS: u32 = 6;
+/// # const binder_driver_return_protocol_BR_ACQUIRE: u32 = 7;
+/// # const binder_driver_return_protocol_BR_RELEASE: u32 = 8;
+/// # const binder_driver_return_protocol_BR_DECREFS: u32 = 9;
+/// # const binder_driver_return_protocol_BR_NOOP: u32 = 10;
+/// # const binder_driver_return_protocol_BR_SPAWN_LOOPER: u32 = 11;
+/// # const binder_driver_return_protocol_BR_DEAD_BINDER: u32 = 12;
+/// # const binder_driver_return_protocol_BR_CLEAR_DEATH_NOTIFICATION_DONE: u32 = 13;
+/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
 /// macro_rules! pub_no_prefix {
 ///     ($prefix:ident, $($newname:ident),+) => {
 ///         kernel::macros::paste! {
-///             $(pub(crate) const fn [<$newname:lower:span>]: u32 = [<$prefix $newname:span>];)+
+///             $(pub(crate) const fn [<$newname:lower:span>]() -> u32 {
+///                 [<$prefix $newname:span>]
+///             })+
 ///         }
 ///     };
 /// }
@@ -349,7 +377,7 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
 ///
 /// Literals can also be concatenated with other identifiers:
 ///
-/// ```ignore
+/// ```rust
 /// macro_rules! create_numbered_fn {
 ///     ($name:literal, $val:literal) => {
 ///         kernel::macros::paste! {
diff --git a/rust/macros/paste.rs b/rust/macros/paste.rs
index f40d42b35b58..6529a387673f 100644
--- a/rust/macros/paste.rs
+++ b/rust/macros/paste.rs
@@ -2,7 +2,7 @@
 
 use proc_macro::{Delimiter, Group, Ident, Spacing, Span, TokenTree};
 
-fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
+fn concat_helper(tokens: &[TokenTree]) -> Vec<(String, Span)> {
     let mut tokens = tokens.iter();
     let mut segments = Vec::new();
     let mut span = None;
@@ -46,12 +46,21 @@ fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
                 };
                 segments.push((value, sp));
             }
-            _ => panic!("unexpected token in paste segments"),
+            Some(TokenTree::Group(group)) if group.delimiter() == Delimiter::None => {
+                let tokens = group.stream().into_iter().collect::<Vec<TokenTree>>();
+                segments.append(&mut concat_helper(tokens.as_slice()));
+            }
+            token => panic!("unexpected token in paste segments: {:?}", token),
         };
     }
 
+    segments
+}
+
+fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
+    let segments = concat_helper(tokens);
     let pasted: String = segments.into_iter().map(|x| x.0).collect();
-    TokenTree::Ident(Ident::new(&pasted, span.unwrap_or(group_span)))
+    TokenTree::Ident(Ident::new(&pasted, group_span))
 }
 
 pub(crate) fn expand(tokens: &mut Vec<TokenTree>) {
-- 
2.44.2

[PATCH v2 4/4] rust: macros: Enable the rest of the tests

[Ethan D. Twardy]

Now that the rusttest target for the macros crate is compiled with the
kernel crate as a dependency, the rest of the rustdoc tests can be
enabled.

Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 3278f7c8aa5e..fcd123b2c093 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -101,7 +101,9 @@ pub fn module(ts: TokenStream) -> TokenStream {
 ///
 /// # Examples
 ///
-/// ```ignore
+/// ```rust
+/// # #[macro_use] extern crate macros;
+/// # #[macro_use] extern crate kernel;
 /// use kernel::error::VTABLE_DEFAULT_ERROR;
 /// use kernel::prelude::*;
 ///
@@ -146,12 +148,27 @@ pub fn vtable(attr: TokenStream, ts: TokenStream) -> TokenStream {
 ///
 /// # Examples
 ///
-/// ```ignore
-/// use kernel::macro::concat_idents;
+/// ```rust
+/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
+/// # const binder_driver_return_protocol_BR_ERROR: u32 = 1;
+/// # const binder_driver_return_protocol_BR_TRANSACTION: u32 = 2;
+/// # const binder_driver_return_protocol_BR_REPLY: u32 = 3;
+/// # const binder_driver_return_protocol_BR_DEAD_REPLY: u32 = 4;
+/// # const binder_driver_return_protocol_BR_TRANSACTION_COMPLETE: u32 = 5;
+/// # const binder_driver_return_protocol_BR_INCREFS: u32 = 6;
+/// # const binder_driver_return_protocol_BR_ACQUIRE: u32 = 7;
+/// # const binder_driver_return_protocol_BR_RELEASE: u32 = 8;
+/// # const binder_driver_return_protocol_BR_DECREFS: u32 = 9;
+/// # const binder_driver_return_protocol_BR_NOOP: u32 = 10;
+/// # const binder_driver_return_protocol_BR_SPAWN_LOOPER: u32 = 11;
+/// # const binder_driver_return_protocol_BR_DEAD_BINDER: u32 = 12;
+/// # const binder_driver_return_protocol_BR_CLEAR_DEATH_NOTIFICATION_DONE: u32 = 13;
+/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
+/// use kernel::macros::concat_idents;
 ///
 /// macro_rules! pub_no_prefix {
 ///     ($prefix:ident, $($newname:ident),+) => {
-///         $(pub(crate) const $newname: u32 = kernel::macros::concat_idents!($prefix, $newname);)+
+///         $(pub(crate) const $newname: u32 = concat_idents!($prefix, $newname);)+
 ///     };
 /// }
 ///
@@ -197,7 +214,9 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
 ///
 /// # Examples
 ///
-/// ```rust,ignore
+/// ```rust
+/// # use std::{sync::Mutex, process::Command};
+/// # use kernel::macros::pin_data;
 /// #[pin_data]
 /// struct DriverData {
 ///     #[pin]
@@ -206,7 +225,15 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
 /// }
 /// ```
 ///
-/// ```rust,ignore
+/// ```rust
+/// # use std::{sync::Mutex, process::Command};
+/// # use core::pin::Pin;
+/// # pub struct Info;
+/// # mod bindings {
+/// #     pub unsafe fn destroy_info(_ptr: *mut super::Info) {}
+/// # }
+/// use kernel::macros::{pin_data, pinned_drop};
+///
 /// #[pin_data(PinnedDrop)]
 /// struct DriverData {
 ///     #[pin]
@@ -221,6 +248,8 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
 ///         unsafe { bindings::destroy_info(self.raw_info) };
 ///     }
 /// }
+///
+/// # fn main() {}
 /// ```
 ///
 /// [`pin_init!`]: ../kernel/macro.pin_init.html
@@ -236,13 +265,20 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
 ///
 /// # Examples
 ///
-/// ```rust,ignore
+/// ```rust
+/// # use macros::{pin_data, pinned_drop};
+/// # use std::{sync::Mutex, process::Command};
+/// # use core::pin::Pin;
+/// # mod bindings {
+/// #     pub struct Info;
+/// #     pub unsafe fn destroy_info(_ptr: *mut Info) {}
+/// # }
 /// #[pin_data(PinnedDrop)]
 /// struct DriverData {
 ///     #[pin]
 ///     queue: Mutex<Vec<Command>>,
 ///     buf: Box<[u8; 1024 * 1024]>,
-///     raw_info: *mut Info,
+///     raw_info: *mut bindings::Info,
 /// }
 ///
 /// #[pinned_drop]
@@ -405,7 +441,9 @@ pub fn paste(input: TokenStream) -> TokenStream {
 ///
 /// # Examples
 ///
-/// ```rust,ignore
+/// ```rust
+/// use kernel::macros::Zeroable;
+///
 /// #[derive(Zeroable)]
 /// pub struct DriverData {
 ///     id: i64,
-- 
2.44.2

Re: [PATCH v2 4/4] rust: macros: Enable the rest of the tests

[Alice Ryhl]

On Mon, Jul 1, 2024 at 1:28 PM Ethan D. Twardy <ethan.twardy@gmail.com> wrote:
>
> Now that the rusttest target for the macros crate is compiled with the
> kernel crate as a dependency, the rest of the rustdoc tests can be
> enabled.
>
> Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

I would drop the newline before `fn main()` here:

> @@ -221,6 +248,8 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
>  ///         unsafe { bindings::destroy_info(self.raw_info) };
>  ///     }
>  /// }
> +///
> +/// # fn main() {}
>  /// ```

Otherwise LGTM.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v2 4/4] rust: macros: Enable the rest of the tests

[Ethan D. Twardy]

On Thu Jul 4, 2024 at 4:43 AM CDT, Alice Ryhl wrote:
> On Mon, Jul 1, 2024 at 1:28 PM Ethan D. Twardy <ethan.twardy@gmail.com> wrote:
> >
> > Now that the rusttest target for the macros crate is compiled with the
> > kernel crate as a dependency, the rest of the rustdoc tests can be
> > enabled.
> >
> > Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>
>
> I would drop the newline before `fn main()` here:
>
> > @@ -221,6 +248,8 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
> >  ///         unsafe { bindings::destroy_info(self.raw_info) };
> >  ///     }
> >  /// }
> > +///
> > +/// # fn main() {}
> >  /// ```

So close! v3 incoming :). Thank you for your review!

Re: [PATCH v2 4/4] rust: macros: Enable the rest of the tests

[Alice Ryhl]

On Thu, Jul 4, 2024 at 4:49 PM Ethan D. Twardy <ethan.twardy@gmail.com> wrote:
>
> On Thu Jul 4, 2024 at 4:43 AM CDT, Alice Ryhl wrote:
> > On Mon, Jul 1, 2024 at 1:28 PM Ethan D. Twardy <ethan.twardy@gmail.com> wrote:
> > >
> > > Now that the rusttest target for the macros crate is compiled with the
> > > kernel crate as a dependency, the rest of the rustdoc tests can be
> > > enabled.
> > >
> > > Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>
> >
> > I would drop the newline before `fn main()` here:
> >
> > > @@ -221,6 +248,8 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
> > >  ///         unsafe { bindings::destroy_info(self.raw_info) };
> > >  ///     }
> > >  /// }
> > > +///
> > > +/// # fn main() {}
> > >  /// ```
>
> So close! v3 incoming :). Thank you for your review!

You're welcome :)

Unfortunately, I'm not really able to review the first patch in this series.

Alice