| Message ID | 20241123222849.350287-2-ojeda@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/3] rust: use the `build_error!` macro, not the hidden function | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Sat, Nov 23, 2024 at 11:29 PM Miguel Ojeda <ojeda@kernel.org> wrote: > > Users were using the hidden exported `kernel::build_error` function > instead of the intended `kernel::build_error!` macro, e.g. see the > previous commit. > > To force to use the macro, move it into the `build_assert` module, > thus making it a compilation error and avoiding a collision in the same > "namespace". Using the function now would require typing the module name > (which is hidden), not just a single character. > > Now attempting to use the function will trigger this error with the > right suggestion by the compiler: > > error[E0423]: expected function, found macro `kernel::build_error` > --> samples/rust/rust_minimal.rs:29:9 > | > 29 | kernel::build_error(); > | ^^^^^^^^^^^^^^^^^^^ not a function > | > help: use `!` to invoke the macro > | > 29 | kernel::build_error!(); > | + > > An alternative would be using an alias, but it would be more complex > and moving it into the module seems right since it belongs there and > reduces the amount of code at the crate root. > > Keep the `#[doc(hidden)]` inside `build_assert` in case the module is > not hidden in the future. > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Alice Ryhl <aliceryhl@google.com> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index bf8d7f841f94..73e33a41ea04 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -32,7 +32,8 @@ > pub mod alloc; > #[cfg(CONFIG_BLOCK)] > pub mod block; > -mod build_assert; > +#[doc(hidden)] > +pub mod build_assert; You could also put #![doc(hidden)] at the top of build_assert.rs to simplify the lib.rs list. Not sure what is best. Alice
On Mon, Nov 25, 2024 at 10:14 AM Alice Ryhl <aliceryhl@google.com> wrote: > > You could also put #![doc(hidden)] at the top of build_assert.rs to > simplify the lib.rs list. Not sure what is best. Yeah, not sure. We used outer attributes for the rest of `doc(hidden)`s, including in the same file, so it is probably best to keep it consistent, unless we decide to move all to inner ones. Thanks! Cheers, Miguel
On Mon, Nov 25, 2024 at 10:23 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Mon, Nov 25, 2024 at 10:14 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > > You could also put #![doc(hidden)] at the top of build_assert.rs to > > simplify the lib.rs list. Not sure what is best. > > Yeah, not sure. We used outer attributes for the rest of > `doc(hidden)`s, including in the same file, so it is probably best to > keep it consistent, unless we decide to move all to inner ones. It might actually make sense to move all of them, I think. I find that they make the list difficult to edit. But feel free to pick whichever one you prefer for this change. My RB applies with or without it :) Alice
diff --git a/rust/kernel/build_assert.rs b/rust/kernel/build_assert.rs index 9e37120bc69c..347ba5ce50f4 100644 --- a/rust/kernel/build_assert.rs +++ b/rust/kernel/build_assert.rs @@ -2,6 +2,9 @@ //! Build-time assert. +#[doc(hidden)] +pub use build_error::build_error; + /// Fails the build if the code path calling `build_error!` can possibly be executed. /// /// If the macro is executed in const context, `build_error!` will panic. @@ -23,10 +26,10 @@ #[macro_export] macro_rules! build_error { () => {{ - $crate::build_error("") + $crate::build_assert::build_error("") }}; ($msg:expr) => {{ - $crate::build_error($msg) + $crate::build_assert::build_error($msg) }}; } @@ -73,12 +76,12 @@ macro_rules! build_error { macro_rules! build_assert { ($cond:expr $(,)?) => {{ if !$cond { - $crate::build_error(concat!("assertion failed: ", stringify!($cond))); + $crate::build_assert::build_error(concat!("assertion failed: ", stringify!($cond))); } }}; ($cond:expr, $msg:expr) => {{ if !$cond { - $crate::build_error($msg); + $crate::build_assert::build_error($msg); } }}; } diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index bf8d7f841f94..73e33a41ea04 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -32,7 +32,8 @@ pub mod alloc; #[cfg(CONFIG_BLOCK)] pub mod block; -mod build_assert; +#[doc(hidden)] +pub mod build_assert; pub mod device; pub mod error; #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)] @@ -66,9 +67,6 @@ pub use macros; pub use uapi; -#[doc(hidden)] -pub use build_error::build_error; - /// Prefix to appear before log messages printed from within the `kernel` crate. const __LOG_PREFIX: &[u8] = b"rust_kernel\0";
Users were using the hidden exported `kernel::build_error` function instead of the intended `kernel::build_error!` macro, e.g. see the previous commit. To force to use the macro, move it into the `build_assert` module, thus making it a compilation error and avoiding a collision in the same "namespace". Using the function now would require typing the module name (which is hidden), not just a single character. Now attempting to use the function will trigger this error with the right suggestion by the compiler: error[E0423]: expected function, found macro `kernel::build_error` --> samples/rust/rust_minimal.rs:29:9 | 29 | kernel::build_error(); | ^^^^^^^^^^^^^^^^^^^ not a function | help: use `!` to invoke the macro | 29 | kernel::build_error!(); | + An alternative would be using an alias, but it would be more complex and moving it into the module seems right since it belongs there and reduces the amount of code at the crate root. Keep the `#[doc(hidden)]` inside `build_assert` in case the module is not hidden in the future. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> --- rust/kernel/build_assert.rs | 11 +++++++---- rust/kernel/lib.rs | 6 ++---- 2 files changed, 9 insertions(+), 8 deletions(-)