Messages in this thread | | | Date | Fri, 27 Oct 2023 09:25:06 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH v3] rust: macros: improve `#[vtable]` documentation |
| |
On 10/27/23 10:02, Finn Behrens wrote: > > > On 26 Oct 2023, at 22:19, Benno Lossin wrote: > >> Traits marked with `#[vtable]` need to provide default implementations >> for optional functions. The C side represents these with `NULL` in the >> vtable, so the default functions are never actually called. We do not >> want to replicate the default behavior from C in Rust, because that is >> not maintainable. Therefore we should use `build_error` in those default >> implementations. The error message for that is provided at >> `kernel::error::VTABLE_DEFAULT_ERROR`. >> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me> >> --- >> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs >> index c42105c2ff96..917a51183c23 100644 >> --- a/rust/macros/lib.rs >> +++ b/rust/macros/lib.rs >> @@ -87,27 +87,48 @@ pub fn module(ts: TokenStream) -> TokenStream { >> /// implementation could just return `Error::EINVAL`); Linux typically use C >> /// `NULL` pointers to represent these functions. >> /// >> -/// This attribute is intended to close the gap. Traits can be declared and >> -/// implemented with the `#[vtable]` attribute, and a `HAS_*` associated constant >> -/// will be generated for each method in the trait, indicating if the implementor >> -/// has overridden a method. >> +/// This attribute closes that gap. A trait can be annotated with the `#[vtable]` attribute. >> +/// Implementers of the trait will then also have to annotate the trait with `#[vtable]`. This >> +/// attribute generates a `HAS_*` 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 `kernel::build_error`, which prevents >> +/// calls to this function at compile time: > In the future it would be nice to have something like `#[default]` or `#[optional]` to automatically derive the implementation.
I brought this up in the discussion on zulip [1]. But Wedson argued that macros make it more magical and less easy to understand. So for the time being, I just wanted to improve the docs.
[1]: https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/.60bool.3A.3Athen.60.20helper/near/395659471
-- Cheers, Benno
| |