Message-ID: <CAJ-ks9kt08wGfTNMfgijeBEGizqQy6UA+wOY+Vt3w=ZqfVXVzQ@mail.gmail.com>
Date: Sun, 4 Jan 2026 18:42:11 -0500
From: Tamir Duberstein <tamird@...il.com>
To: Gary Guo <gary@...yguo.net>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Guilherme Giacomo Simoes <trintaeoitogc@...il.com>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 09/11] rust: macros: allow arbitrary types to be used in
 `module!` macro

On Thu, Dec 11, 2025 at 2:30 PM Gary Guo <gary@...nel.org> wrote:
>
> From: Gary Guo <gary@...yguo.net>
>
> Previously this only accepts an identifier, but now with `syn` it is
> easy to make it accepts any type.
>
> With this change, the span of types are preserved -- as a benefit, Rust
> analyzer will be able to use the "navigate to definition" feature on
> type name inside `module!` macro invocation.
>
> Signed-off-by: Gary Guo <gary@...yguo.net>
> ---
>  rust/macros/module.rs | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/rust/macros/module.rs b/rust/macros/module.rs
> index 4a02aadef25a7..d6298d04c86f4 100644
> --- a/rust/macros/module.rs
> +++ b/rust/macros/module.rs
> @@ -4,7 +4,6 @@
>
>  use proc_macro2::{
>      Literal,
> -    Span,
>      TokenStream, //
>  };
>  use quote::{
> @@ -23,7 +22,8 @@
>      Ident,
>      LitStr,
>      Result,
> -    Token, //
> +    Token,
> +    Type, //
>  };
>
>  use crate::helpers::*;
> @@ -105,7 +105,7 @@ mod kw {
>
>  #[allow(dead_code, reason = "some fields are only parsed into")]
>  enum ModInfoField {
> -    Type(Token![type], Token![:], Ident),
> +    Type(Token![type], Token![:], Type),
>      Name(kw::name, Token![:], AsciiLitStr),
>      Authors(
>          kw::authors,
> @@ -193,9 +193,9 @@ fn parse(input: ParseStream<'_>) -> Result<Self> {
>      }
>  }
>
> -#[derive(Debug, Default)]
> +#[derive(Default)]
>  pub(crate) struct ModuleInfo {
> -    type_: String,
> +    type_: Option<Type>,
>      license: String,
>      name: String,
>      authors: Option<Vec<String>>,
> @@ -237,7 +237,7 @@ fn parse(input: ParseStream<'_>) -> Result<Self> {
>              seen_keys.push(key);
>
>              match field {
> -                ModInfoField::Type(_, _, ty) => info.type_ = ty.to_string(),
> +                ModInfoField::Type(_, _, ty) => info.type_ = Some(ty),
>                  ModInfoField::Name(_, _, name) => info.name = name.value(),
>                  ModInfoField::Authors(_, _, _, list) => {
>                      info.authors = Some(list.into_iter().map(|x| x.value()).collect())
> @@ -286,16 +286,17 @@ fn parse(input: ParseStream<'_>) -> Result<Self> {
>
>  pub(crate) fn module(info: ModuleInfo) -> Result<TokenStream> {
>      let ModuleInfo {
> -        type_,
> +        type_: Some(type_),
>          license,
>          name,
>          authors,
>          description,
>          alias,
>          firmware,
> -    } = info;
> -
> -    let type_ = Ident::new(&type_, Span::mixed_site());
> +    } = info
> +    else {
> +        unreachable!();
> +    };

I'm not a huge fan of this. As far as I can tell it arises from the
use of Default+mutation. It would be better, I think, to make type_
non-optional instead (and avoid using Default to construct a
partially-initialized ModuleInfo).

>
>      // Rust does not allow hyphens in identifiers, use underscore instead.
>      let ident = name.replace('-', "_");
> @@ -376,7 +377,6 @@ impl ::kernel::ModuleMetadata for #type_ {
>          // Double nested modules, since then nobody can access the public items inside.
>          mod __module_init {
>              mod __module_init {
> -                use super::super::#type_;
>                  use pin_init::PinInit;
>
>                  /// The "Rust loadable module" mark.
> @@ -388,7 +388,7 @@ mod __module_init {
>                  #[used(compiler)]
>                  static __IS_RUST_MODULE: () = ();
>
> -                static mut __MOD: ::core::mem::MaybeUninit<#type_> =
> +                static mut __MOD: ::core::mem::MaybeUninit<super::super::LocalModule> =
>                      ::core::mem::MaybeUninit::uninit();
>
>                  // Loadable modules need to export the `{init,cleanup}_module` identifiers.
> @@ -475,8 +475,9 @@ pub extern "C" fn #ident_exit() {
>                  ///
>                  /// This function must only be called once.
>                  unsafe fn __init() -> ::kernel::ffi::c_int {
> -                    let initer =
> -                        <#type_ as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE);
> +                    let initer = <super::super::LocalModule as ::kernel::InPlaceModule>::init(
> +                        &super::super::THIS_MODULE
> +                    );
>                      // SAFETY: No data race, since `__MOD` can only be accessed by this module
>                      // and there only `__init` and `__exit` access it. These functions are only
>                      // called once and `__exit` cannot be called before or during `__init`.
> --
> 2.51.2
>

Powered by blists - more mailing lists