Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
to consider:
- [Low] Deprecated `+` syntax fallback rule for relative register aliases
incorrectly captures the alias offset as a `ty` fragment, causing a macro
expansion error.
- [Low] Mixing the deprecated `+` syntax and the new `:` syntax within the same
`register! { ... }` block results in a compilation error.
--
--- Patch [5]: [PATCH 5/8] rust: io: register: allow paths for relative
register bases ---
commit 0d4940e7a464ea733ee8d4db86d88c71b931bf2d
Author: Alexandre Courbot <[email protected]>
rust: io: register: allow paths for relative register bases
This commit introduces Base: Offset and Base: Alias syntax in the register!
macro, parsing them as path fragments rather than ident. It temporarily
retains the deprecated + syntax fallback to allow in-tree users to be
converted incrementally in subsequent patches.
> diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
> --- a/rust/kernel/io/register.rs
> +++ b/rust/kernel/io/register.rs
> @@ -801,6 +801,26 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) {
> /// ```
> #[macro_export]
> macro_rules! register {
> + // Deprecated `+` syntax for relative registers and their aliases.
> + (
> + $(
> + $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
> + $([ $size:expr $(, stride = $stride:expr)? ])?
> + $(@ $($base:ident +)? $offset:literal)?
> + $(=> $alias:ident $(+ $alias_offset:ty)?
> $([$alias_idx:expr])? )?
^^^
[Severity: Low]
Does capturing the alias offset as a ty fragment here break the backward
compatibility?
When this is forwarded to the internal rule:
@reg $(#[$attr])* $vis $name ($storage) => $base:path : $alias:path
a parsed ty fragment cannot be matched by a path fragment specifier in the
subsequent macro invocation. This seems to result in a "no rules expected the
token in macro call" compilation error for any code still using the old
syntax.
[ ... ]
> // Entry point for the macro, allowing multiple registers to be defined
> in one call.
> // It matches all possible register declaration patterns to dispatch
> them to corresponding
> // `@reg` rule that defines a single register.
> @@ -808,8 +828,8 @@ macro_rules! register {
> $(
> $(#[$attr:meta])* $vis:vis $name:ident ($storage:ty)
> $([ $size:expr $(, stride = $stride:expr)? ])?
> - $(@ $($base:ident +)? $offset:literal)?
> - $(=> $alias:ident $(+ $alias_offset:path)?
> $([$alias_idx:expr])? )?
> + $(@ $($base:path :)? $offset:literal)?
> + $(=> $alias:path $(: $alias_offset:path)?
> $([$alias_idx:expr])? )?
> { $($fields:tt)* }
> )*
> ) => {
[Severity: Low]
Will this structure prevent mixing the old and new syntax within the same
register block?
Since both top-level macro arms use $( ... )* to match the entire contents
of the block, a block containing a mix of + and : syntax will fail both
matchers. This might cause compilation errors if users attempt to
incrementally migrate some, but not all, registers within a single
register block.
--
Sashiko AI review ยท
https://sashiko.dev/#/patchset/[email protected]?part=5
