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

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

[Ethan D. Twardy]

This patch series addresses GitHub issue Rust-For-Linux/linux#1076 by
enabling the rustdoc tests for the macros crate and updating kbuild to
compile these tests with appropriate dependencies respected. Reviewers:
Please pay particular attention to the Kbuild changes in rust/Makefile.
I appreciate your time in reviewing this series!

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        |  37 ++++++++++++-
 rust/macros/lib.rs   | 127 ++++++++++++++++++++++++++++++++-----------
 rust/macros/paste.rs |  15 ++++-
 3 files changed, 142 insertions(+), 37 deletions(-)


base-commit: a126eca844353360ebafa9088d22865cb8e022e3
-- 
2.44.2

[PATCH 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..de58f0cae23b 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -147,6 +147,23 @@ rusttestlib-macros: private rustc_test_library_proc = yes
 rusttestlib-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
 	+$(call if_changed,rustc_test_library)
 
+rusttestlib-build_error: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE
+	+$(call if_changed,rustc_test_library)
+
+rusttestlib-uapi: $(src)/uapi/lib.rs \
+    $(obj)/compiler_builtins.o \
+    $(obj)/uapi/uapi_generated.rs 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 +262,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

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

[kernel test robot]

Hi Ethan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on a126eca844353360ebafa9088d22865cb8e022e3]

url:    https://github.com/intel-lab-lkp/linux/commits/Ethan-D-Twardy/kbuild-rust-Expand-rusttest-target-for-macros/20240625-230535
base:   a126eca844353360ebafa9088d22865cb8e022e3
patch link:    https://lore.kernel.org/r/20240624030327.90301-2-ethan.twardy%40gmail.com
patch subject: [PATCH 1/4] kbuild: rust: Expand rusttest target for macros
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240626/202406261127.VzakF8rL-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240626/202406261127.VzakF8rL-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406261127.VzakF8rL-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> rust/Makefile:151: warning: overriding recipe for target 'rusttestlib-build_error'
>> rust/Makefile:143: warning: ignoring old recipe for target 'rusttestlib-build_error'
>> rust/Makefile:171: warning: overriding recipe for target 'rusttestlib-uapi'
>> rust/Makefile:156: warning: ignoring old recipe for target 'rusttestlib-uapi'


vim +151 rust/Makefile

    59	
    60	core-cfgs = \
    61	    --cfg no_fp_fmt_parse
    62	
    63	alloc-cfgs = \
    64	    --cfg no_global_oom_handling \
    65	    --cfg no_rc \
    66	    --cfg no_sync
    67	
    68	quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
    69	      cmd_rustdoc = \
    70		OBJTREE=$(abspath $(objtree)) \
    71		$(RUSTDOC) $(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) \
    72			$(rustc_target_flags) -L$(objtree)/$(obj) \
    73			--output $(rustdoc_output) \
    74			--crate-name $(subst rustdoc-,,$@) \
    75			$(if $(rustdoc_host),,--sysroot=/dev/null) \
    76			@$(objtree)/include/generated/rustc_cfg $<
    77	
    78	# The `html_logo_url` and `html_favicon_url` forms of the `doc` attribute
    79	# can be used to specify a custom logo. However:
    80	#   - The given value is used as-is, thus it cannot be relative or a local file
    81	#     (unlike the non-custom case) since the generated docs have subfolders.
    82	#   - It requires adding it to every crate.
    83	#   - It requires changing `core` which comes from the sysroot.
    84	#
    85	# Using `-Zcrate-attr` would solve the last two points, but not the first.
    86	# The https://github.com/rust-lang/rfcs/pull/3226 RFC suggests two new
    87	# command-like flags to solve the issue. Meanwhile, we use the non-custom case
    88	# and then retouch the generated files.
    89	rustdoc: rustdoc-core rustdoc-macros rustdoc-compiler_builtins \
    90	    rustdoc-alloc rustdoc-kernel
    91		$(Q)cp $(srctree)/Documentation/images/logo.svg $(rustdoc_output)/static.files/
    92		$(Q)cp $(srctree)/Documentation/images/COPYING-logo $(rustdoc_output)/static.files/
    93		$(Q)find $(rustdoc_output) -name '*.html' -type f -print0 | xargs -0 sed -Ei \
    94			-e 's:rust-logo-[0-9a-f]+\.svg:logo.svg:g' \
    95			-e 's:favicon-[0-9a-f]+\.svg:logo.svg:g' \
    96			-e 's:<link rel="alternate icon" type="image/png" href="[/.]+/static\.files/favicon-(16x16|32x32)-[0-9a-f]+\.png">::g' \
    97			-e 's:<a href="srctree/([^"]+)">:<a href="$(realpath $(srctree))/\1">:g'
    98		$(Q)for f in $(rustdoc_output)/static.files/rustdoc-*.css; do \
    99			echo ".logo-container > img { object-fit: contain; }" >> $$f; done
   100	
   101	rustdoc-macros: private rustdoc_host = yes
   102	rustdoc-macros: private rustc_target_flags = --crate-type proc-macro \
   103	    --extern proc_macro
   104	rustdoc-macros: $(src)/macros/lib.rs FORCE
   105		+$(call if_changed,rustdoc)
   106	
   107	rustdoc-core: private rustc_target_flags = $(core-cfgs)
   108	rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE
   109		+$(call if_changed,rustdoc)
   110	
   111	rustdoc-compiler_builtins: $(src)/compiler_builtins.rs rustdoc-core FORCE
   112		+$(call if_changed,rustdoc)
   113	
   114	# We need to allow `rustdoc::broken_intra_doc_links` because some
   115	# `no_global_oom_handling` functions refer to non-`no_global_oom_handling`
   116	# functions. Ideally `rustdoc` would have a way to distinguish broken links
   117	# due to things that are "configured out" vs. entirely non-existing ones.
   118	rustdoc-alloc: private rustc_target_flags = $(alloc-cfgs) \
   119	    -Arustdoc::broken_intra_doc_links
   120	rustdoc-alloc: $(RUST_LIB_SRC)/alloc/src/lib.rs rustdoc-core rustdoc-compiler_builtins FORCE
   121		+$(call if_changed,rustdoc)
   122	
   123	rustdoc-kernel: private rustc_target_flags = --extern alloc \
   124	    --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \
   125	    --extern bindings --extern uapi
   126	rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \
   127	    rustdoc-compiler_builtins rustdoc-alloc $(obj)/libmacros.so \
   128	    $(obj)/bindings.o FORCE
   129		+$(call if_changed,rustdoc)
   130	
   131	quiet_cmd_rustc_test_library = RUSTC TL $<
   132	      cmd_rustc_test_library = \
   133		OBJTREE=$(abspath $(objtree)) \
   134		$(RUSTC) $(rust_common_flags) \
   135			@$(objtree)/include/generated/rustc_cfg $(rustc_target_flags) \
   136			--crate-type $(if $(rustc_test_library_proc),proc-macro,rlib) \
   137			--out-dir $(objtree)/$(obj)/test --cfg testlib \
   138			--sysroot $(objtree)/$(obj)/test/sysroot \
   139			-L$(objtree)/$(obj)/test \
   140			--crate-name $(subst rusttest-,,$(subst rusttestlib-,,$@)) $<
   141	
   142	rusttestlib-build_error: $(src)/build_error.rs rusttest-prepare FORCE
 > 143		+$(call if_changed,rustc_test_library)
   144	
   145	rusttestlib-macros: private rustc_target_flags = --extern proc_macro
   146	rusttestlib-macros: private rustc_test_library_proc = yes
   147	rusttestlib-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
   148		+$(call if_changed,rustc_test_library)
   149	
   150	rusttestlib-build_error: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE
 > 151		+$(call if_changed,rustc_test_library)
   152	
   153	rusttestlib-uapi: $(src)/uapi/lib.rs \
   154	    $(obj)/compiler_builtins.o \
   155	    $(obj)/uapi/uapi_generated.rs FORCE
 > 156		+$(call if_changed,rustc_test_library)
   157	
   158	rusttestlib-kernel: private rustc_target_flags = --extern alloc \
   159	    --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \
   160	    --extern bindings --extern uapi
   161	rusttestlib-kernel: $(src)/kernel/lib.rs rustdoc-compiler_builtins \
   162	    rustdoc-alloc rusttestlib-bindings rusttestlib-uapi rusttestlib-build_error \
   163	    $(obj)/libmacros.so \
   164	    $(obj)/bindings.o FORCE
   165		+$(call if_changed,rustc_test_library)
   166	
   167	rusttestlib-bindings: $(src)/bindings/lib.rs rusttest-prepare FORCE
   168		+$(call if_changed,rustc_test_library)
   169	
   170	rusttestlib-uapi: $(src)/uapi/lib.rs rusttest-prepare FORCE
 > 171		+$(call if_changed,rustc_test_library)
   172	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 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.

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

diff --git a/rust/Makefile b/rust/Makefile
index de58f0cae23b..16abb8175494 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -264,6 +264,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..d8bd34c0ba89 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,15 @@
 ///     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

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

[Alice Ryhl]

On Mon, Jun 24, 2024 at 5:04 AM Ethan D. Twardy <ethan.twardy@gmail.com> wrote:
>
> Previously, this test was ignored due to a missing necessary dependency
> on the `kernel` crate.
>
> Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

It may be worth to call out in the commit message that you are
changing what the example does. (But the change is good - this form of
kernel parameters were never upstreamed.)

With the commit message updated:

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

Oh, also, you don't want an empty line between } and `# fn main() {}`
as it will show up as a weird empty line at the end of the example.

Alice

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

[Ethan D. Twardy]

On Mon Jun 24, 2024 at 3:32 AM CDT, Alice Ryhl wrote:
> It may be worth to call out in the commit message that you are
> changing what the example does. (But the change is good - this form of
> kernel parameters were never upstreamed.)
>
> With the commit message updated:
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> Oh, also, you don't want an empty line between } and `# fn main() {}`
> as it will show up as a weird empty line at the end of the example.
>
> Alice

I have these changes made locally and will send with v2. Thank you!

[PATCH 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.

[1]: https://doc.rust-lang.org/stable/proc_macro/enum.Delimiter.html

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

diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index d8bd34c0ba89..8afed8facb21 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -269,12 +269,26 @@ 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>];)+
 ///         }
 ///     };
@@ -313,13 +327,29 @@ 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>]
+///             })+
 ///         }
 ///     };
 /// }
@@ -350,7 +380,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

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

[Alice Ryhl]

Ethan D. Twardy <ethan.twardy@gmail.com> writes:
> 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.

The actual change looks good to me.

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

However, I have a bunch of formatting nits:

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

Normally this would be formatted as:

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

> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index d8bd34c0ba89..8afed8facb21 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -269,12 +269,26 @@ 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),+) => {

There's a non-hidden empty line between the last constant and
`macro_rules! pub_no_prefix`. You should either hide the empty line or
get rid of it, because it will look weird when the example is rendered.

> -///         paste! {
> +///         kernel::macros::paste! {

Another option would be to keep the import so that the empty line
separates the import from the macro declaration.

>  ///             $(pub(crate) const $newname: u32 = [<$prefix $newname>];)+
>  ///         }
>  ///     };
> @@ -313,13 +327,29 @@ 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),+) => {

Same here.

>  ///         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>]
> +///             })+

I would probably indent [<$prefix $newname:span>] one more time.

Alice

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

[Ethan D. Twardy]

On Mon Jun 24, 2024 at 3:43 AM CDT, Alice Ryhl wrote:
> The actual change looks good to me.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>

Thank you!

> Normally this would be formatted as:
>
> Link: https://doc.rust-lang.org/stable/proc_macro/enum.Delimiter.html [1]
> Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

Ah, thank you for informing me on this :).

> There's a non-hidden empty line between the last constant and
> `macro_rules! pub_no_prefix`. You should either hide the empty line or
> get rid of it, because it will look weird when the example is rendered.
>
>
> Another option would be to keep the import so that the empty line
> separates the import from the macro declaration.

Done! (And the one other identical one, as well).

> I would probably indent [<$prefix $newname:span>] one more time.
>
> Alice

Done, as well. As with PATCH 2/4, these will ship with v2.

[PATCH 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 8afed8facb21..6d764099563b 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -102,7 +102,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::*;
 ///
@@ -147,12 +149,28 @@ 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);)+
 ///     };
 /// }
 ///
@@ -198,8 +216,9 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
 ///
 /// # Examples
 ///
-/// ```rust,ignore
-/// #[pin_data]
+/// ```rust
+/// # use std::{sync::Mutex, process::Command};
+/// #[kernel::macros::pin_data]
 /// struct DriverData {
 ///     #[pin]
 ///     queue: Mutex<Vec<Command>>,
@@ -207,7 +226,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]
@@ -222,6 +249,8 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
 ///         unsafe { bindings::destroy_info(self.raw_info) };
 ///     }
 /// }
+///
+/// # fn main() {}
 /// ```
 ///
 /// [`pin_init!`]: ../kernel/macro.pin_init.html
@@ -237,13 +266,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]
@@ -408,7 +444,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 4/4] rust: macros: Enable the rest of the tests

[Alice Ryhl]

On Mon, Jun 24, 2024 at 5:04 AM 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>
>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 8afed8facb21..6d764099563b 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -102,7 +102,9 @@ pub fn module(ts: TokenStream) -> TokenStream {
>  ///
>  /// # Examples
>  ///
> -/// ```ignore
> +/// ```rust
> +/// # #[macro_use] extern crate macros;
> +/// # #[macro_use] extern crate kernel;

You also added these lines in patch 2, but you did not make them hidden there.

>  /// use kernel::error::VTABLE_DEFAULT_ERROR;
>  /// use kernel::prelude::*;
>  ///
> @@ -147,12 +149,28 @@ 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;

The empty line above this import should probably be removed to improve
how this is rendered.

>  ///
>  /// 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);)+
>  ///     };
>  /// }
>  ///
> @@ -198,8 +216,9 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
>  ///
>  /// # Examples
>  ///
> -/// ```rust,ignore
> -/// #[pin_data]
> +/// ```rust
> +/// # use std::{sync::Mutex, process::Command};
> +/// #[kernel::macros::pin_data]
>  /// struct DriverData {

I'm pretty sure `#[pin_data]` is in our prelude and doesn't need an
import in normal code. If it needs an import in the test, then please
add a hidden import rather than using the full path.

>  ///     #[pin]
>  ///     queue: Mutex<Vec<Command>>,
> @@ -207,7 +226,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]
> @@ -222,6 +249,8 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream {
>  ///         unsafe { bindings::destroy_info(self.raw_info) };
>  ///     }
>  /// }
> +///
> +/// # fn main() {}
>  /// ```
>  ///
>  /// [`pin_init!`]: ../kernel/macro.pin_init.html
> @@ -237,13 +266,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]
> @@ -408,7 +444,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 4/4] rust: macros: Enable the rest of the tests

[Ethan D. Twardy]

On Mon Jun 24, 2024 at 3:47 AM CDT, Alice Ryhl wrote:
> On Mon, Jun 24, 2024 at 5:04 AM 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>
> >
> > diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> > index 8afed8facb21..6d764099563b 100644
> > --- a/rust/macros/lib.rs
> > +++ b/rust/macros/lib.rs
> > @@ -102,7 +102,9 @@ pub fn module(ts: TokenStream) -> TokenStream {
> >  ///
> >  /// # Examples
> >  ///
> > -/// ```ignore
> > +/// ```rust
> > +/// # #[macro_use] extern crate macros;
> > +/// # #[macro_use] extern crate kernel;
>
> You also added these lines in patch 2, but you did not make them hidden there.
>

I made these hidden in patch 2, since I would not expect them to be
necessary in normal kernel code. Please let me know if that's not the
case!

> The empty line above this import should probably be removed to improve
> how this is rendered.

And

> I'm pretty sure `#[pin_data]` is in our prelude and doesn't need an
> import in normal code. If it needs an import in the test, then please
> add a hidden import rather than using the full path.

Will also land in v2. Thank you very much for your time in reviewing
this patch series! Did you intend to offer your Reviewed-by signoff for
this patch (4/4) as well?

Ethan