[PATCH 2/3] rust: macros: vtable: add `#[optional]` attribute

Gary Guo <[email protected]>
Newsgroups org.kernel.vger.rust-for-linux,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
For most users of the `#[vtable]` macro, the optional methods do not have a
Rust-side default implementation; a `NULL` pointer is inserted to the
vtable and the C subsystem does thing differently when it sees `NULL`.

Streamline this use case by providing a `#[optional]` attribute that does
what the `build_error!(VTABLE_DEFAULT_ERROR)` boilerplate does.

Signed-off-by: Gary Guo <[email protected]>
---
 rust/macros/lib.rs    | 39 +++++++++++----------------------------
 rust/macros/vtable.rs | 26 +++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 0514fc7c0a55..de55dee4d866 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -156,44 +156,29 @@ pub fn module(input: TokenStream) -> TokenStream {
 /// associated constant bool for each method in the trait that is set to true if
 /// the implementer has overridden the associated method.
 ///
-/// For a trait method to be optional, it must have a default implementation.
-/// This is also the case for traits annotated with `#[vtable]`, but in this
-/// case the default implementation will never be executed. The reason for this
-/// is that the functions will be called through function pointers installed in
-/// C side vtables. When an optional method is not implemented on a `#[vtable]`
-/// trait, a `NULL` entry is installed in the vtable. Thus the default
-/// implementation is never called. Since these traits are not designed to be
-/// used on the Rust side, it should not be possible to call the default
-/// implementation. This is done to ensure that we call the vtable methods
-/// through the C vtable, and not through the Rust vtable. Therefore, the
-/// default implementation should call `build_error!`, which prevents
-/// calls to this function at compile time:
-///
-/// ```compile_fail
-/// # // Intentionally missing `use`s to simplify `rusttest`.
-/// build_error!(VTABLE_DEFAULT_ERROR)
-/// ```
-///
-/// Note that you might need to import [`kernel::error::VTABLE_DEFAULT_ERROR`].
+/// For a trait method to be optional for normal traits, it must have a default implementation.
+/// However, for many users of `#[vtable]`, the functions will be called through function pointers
+/// installed in C side vtables. When an optional method is not implemented on a `#[vtable]` trait,
+/// a `NULL` entry is installed in the vtable; thus the default implementation is never called. If
+/// this is the case, `#[optional]` can be applied on the optional method. Methods annotated as such
+/// do not need to be implemented nor need a default implementation. Calling these methods on types
+/// that do not implement them will fail to build using the `build_error!` mechanism.
 ///
 /// This macro should not be used when all functions are required.
 ///
 /// # Examples
 ///
 /// ```
-/// use kernel::error::VTABLE_DEFAULT_ERROR;
 /// use kernel::prelude::*;
 ///
 /// // Declares a `#[vtable]` trait
 /// #[vtable]
 /// pub trait Operations: Send + Sync + Sized {
-///     fn foo(&self) -> Result<()> {
-///         build_error!(VTABLE_DEFAULT_ERROR)
-///     }
+///     #[optional]
+///     fn foo(&self) -> Result<()>;
 ///
-///     fn bar(&self) -> Result<()> {
-///         build_error!(VTABLE_DEFAULT_ERROR)
-///     }
+///     #[optional]
+///     fn bar(&self) -> Result<()>;
 /// }
 ///
 /// struct Foo;
@@ -210,8 +195,6 @@ pub fn module(input: TokenStream) -> TokenStream {
 /// assert_eq!(<Foo as Operations>::HAS_FOO, true);
 /// assert_eq!(<Foo as Operations>::HAS_BAR, false);
 /// ```
-///
-/// [`kernel::error::VTABLE_DEFAULT_ERROR`]: ../kernel/error/constant.VTABLE_DEFAULT_ERROR.html
 #[proc_macro_attribute]
 pub fn vtable(attr: TokenStream, input: TokenStream) -> TokenStream {
     parse_macro_input!(attr as syn::parse::Nothing);
diff --git a/rust/macros/vtable.rs b/rust/macros/vtable.rs
index e28976a5919f..54bd32e82a9c 100644
--- a/rust/macros/vtable.rs
+++ b/rust/macros/vtable.rs
@@ -33,7 +33,7 @@ fn handle_trait(mut item: ItemTrait) -> Result<ItemTrait> {
          const USE_VTABLE_ATTR: ();
     });
 
-    for item in &item.items {
+    for item in &mut item.items {
         if let TraitItem::Fn(fn_item) = item {
             let name = &fn_item.sig.ident;
             let gen_const_name = Ident::new(
@@ -41,6 +41,30 @@ fn handle_trait(mut item: ItemTrait) -> Result<ItemTrait> {
                 name.span(),
             );
 
+            if fn_item
+                .attrs
+                .extract_if(.., |attr| attr.path().is_ident("optional"))
+                .count()
+                != 0
+            {
+                if let Some(default) = &fn_item.default {
+                    Err(Error::new_spanned(
+                        default,
+                        "`#[optional]` methods must not have default implementation",
+                    ))?;
+                }
+
+                // Optional methods in Rust need a default implementation. Inject one that fails
+                // the build in compile-time if not overridden.
+                fn_item.default = Some(parse_quote!({
+                    ::kernel::build_assert::build_error!(
+                        "This function must not be called, see the #[vtable] documentation."
+                    );
+                }));
+                // Ensure that the function is never code generated unless used.
+                fn_item.attrs.push(parse_quote!(#[inline]));
+            }
+
             // We don't know on the implementation-site whether a method is required or provided
             // so we have to generate a const for all methods.
             // However, hide it for required methods as it will always be true.

-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.