Messages in this thread | | | Date | Wed, 25 Oct 2023 21:34:02 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH v2] rust: macros: improve `#[vtable]` documentation |
| |
On 25.10.23 21:14, Gary Guo wrote: > On Tue, 24 Oct 2023 14:43:30 +0000 > Benno Lossin <benno.lossin@proton.me> wrote: > >> On 24.10.23 13:24, Gary Guo wrote: >>> On Thu, 19 Oct 2023 17:15:53 +0000 >>> Benno Lossin <benno.lossin@proton.me> wrote: >> >> [...] >> >>>> -/// 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 function to be optional, it must have a default implementation. But this default >>>> +/// implementation will never be executed, since these functions are exclusively called from >>>> +/// callbacks from the C side. This is because the vtable will have a `NULL` entry and the C side >>>> +/// will execute the default behavior. Since it is not maintainable to replicate the default >>>> +/// behavior in Rust, the default implementation should be: >>>> +/// >>>> +/// ```compile_fail >>>> +/// # use kernel::error::VTABLE_DEFAULT_ERROR; >>>> +/// kernel::build_error(VTABLE_DEFAULT_ERROR) >>> >>> Note that `build_error` function is considered impl detail and is >>> hidden. >> >> I see, we should mention that in the docs on `build_error`. > > Well, it's marked as `#[doc(hidden)]`...
Yes, but I did not even build the docs, but read it directly inside of the `build_error` crate and thus I did not see the `#[doc(hidden)]`.
>>> This should use the macro version instead: >>> >>> kernel::build_error!(VTABLE_DEFAULT_ERROR) >> >> Is there a reason that it is a macro? Why is it re-exported in the >> kernel crate? The macro could just use `::bulid_error::build_error()`. > > The original intention is to allow format strings, but Rust const > panic is not powerful enough to support it at the moment. Macro > allows higher flexibility.
That is what I thought. But should we then not always require a string literal? So
kernel::build_error!("{}", VTABLE_DEFAULT_ERROR)
> It's re-exported so the macro can reference them (note that downstream > crates can't reference build_error directly). Perhaps I should put it > inside __private_reexport or something instead.
I see, I did not know that they cannot reference build error directly. Is that some limitation of the build system? If it is possible to not re-export it, I think that would be better, otherwise just put it in `__private`.
-- Cheers, Benno
| |