# Meeting 2024-04-10 <!-- Leave your topic starting with ### in the relevant sections below --> ## Critical <!-- bugs, soundness issues, urgent patches/reviews etc. --> ## Status Reports <!-- You want to report on something you are working on/request reviews. Or you want to know the status of something someone else is doing --> ### rewriting our macros using `syn` and `quote` Miguel has updated the patches that introduce `syn` and `quote`. I have rewritten our proc-macros (and also the `__init_interal!` macro behind the `[try_][pin_]init!` macros) to use `syn` and `quote`. Here is a small summary: ```patch Benno Lossin (9): rust: macros: replace `quote!` with `quote::quote` and use `proc-macro2` rust: macros: rewrite `#[vtable]` using `syn` rust: macros: rewrite `module!` using `syn` rust: macros: rewrite `Zeroable` derive macro using `syn` rust: macros: rewrite `#[pin_data]` using `syn` rust: macros: rewrite `#[pinned_drop]` using `syn` rust: macros: rewrite `__internal_init!` using `syn` rust: macros: remove helpers rust: init: remove macros.rs rust/kernel/init.rs | 80 +- rust/kernel/init/macros.rs | 1400 --------------------------------- rust/macros/concat_idents.rs | 11 +- rust/macros/helpers.rs | 217 ----- rust/macros/lib.rs | 85 +- rust/macros/module.rs | 674 +++++++++------- rust/macros/paste.rs | 2 +- rust/macros/pin_data.rs | 407 +++++++--- rust/macros/pinned_drop.rs | 141 +++- rust/macros/primitive_init.rs | 390 +++++++++ rust/macros/quote.rs | 157 ---- rust/macros/vtable.rs | 129 ++- rust/macros/zeroable.rs | 122 ++- 13 files changed, 1387 insertions(+), 2428 deletions(-) delete mode 100644 rust/kernel/init/macros.rs delete mode 100644 rust/macros/helpers.rs create mode 100644 rust/macros/primitive_init.rs delete mode 100644 rust/macros/quote.rs ``` Not only the line count is much better, but also the macros are a lot more readable. For example, the new `primitive_init!` macro that replaces `__init_interal!` does not need to use token munching. Take a look at the patches [here](https://github.com/Rust-for-Linux/linux/pull/1073). I have also deleted `rust/kernel/init/macros.rs`, since the only thing left in there was a documented expansion of the `#[pin_data]` and `__internal_init!` macros, but I don't know if we want to keep it (since the `syn` code should be readable enough) or where to move it. #### Better error handling Using `syn`, several errors become much nicer. Here a are couple of examples. ##### Adding an additional `,` at the end of `..Zeroable::zeroed()` ```rust pin_init!(Foo { a: Bar, b <- Bar, c: Bar, d: Bar, ..Zeroable::zeroed(), // ^ this is the culprit! }) ``` ```text error: no rules expected the token `,` --> samples/rust/rust_minimal.rs:51:5 | 51 | / pin_init!(Foo { 52 | | a: Bar, 53 | | b <- Bar, 54 | | c: Bar, 55 | | d: Bar, 56 | | ..Zeroable::zeroed(), 57 | | }) | |______^ no rules expected this token in macro call | note: while trying to match `)` --> rust/kernel/init/macros.rs:1169:53 | 1169 | @munch_fields($(..Zeroable::zeroed())? $(,)?), | ^ = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `pin_init` (in Nightly builds, run with -Z macro-backtrace for more info) error: no rules expected the token `,` --> samples/rust/rust_minimal.rs:51:5 | 51 | / pin_init!(Foo { 52 | | a: Bar, 53 | | b <- Bar, 54 | | c: Bar, 55 | | d: Bar, 56 | | ..Zeroable::zeroed(), 57 | | }) | |______^ no rules expected this token in macro call | note: while trying to match `)` --> rust/kernel/init/macros.rs:1273:49 | 1273 | @munch_fields(..Zeroable::zeroed() $(,)?), | ^ = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `pin_init` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 2 previous errors ``` Now just is this: ```text error: unexpected token --> samples/rust/rust_minimal.rs:56:29 | 56 | ..Zeroable::zeroed(), | ^ error: aborting due to 1 previous error ``` ##### Forgetting to add a comma between field initializers ```rust pin_init!(Foo { a: Bar, b <- Bar, c: Bar // ^ no comma here d: Bar, }) ``` ```text error: no rules expected the token `d` --> samples/rust/rust_minimal.rs:55:9 | 55 | d: Bar, | ^ no rules expected this token in macro call | note: while trying to match `,` --> rust/kernel/init/macros.rs:1243:51 | 1243 | @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*), | ^ error: no rules expected the token `d` --> samples/rust/rust_minimal.rs:55:9 | 55 | d: Bar, | ^ no rules expected this token in macro call | note: while trying to match `,` --> rust/kernel/init/macros.rs:1338:51 | 1338 | @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*), | ^ error: aborting due to 2 previous errors ``` Also gets much simpler: ```text error: expected `,` --> samples/rust/rust_minimal.rs:55:9 | 55 | d: Bar, | ^ error: aborting due to 1 previous error ``` ##### When forgetting to use `<-` with initializers ```rust pin_init!(Foo { a: Bar, b: init, // ^ this should be `<-` instead c: Bar, d: Bar, }) ``` ```text error[E0308]: mismatched types --> samples/rust/rust_minimal.rs:51:5 | 50 | fn foo(init: impl PinInit<Bar>) -> impl PinInit<Foo> { | ----------------- found this type parameter 51 | / pin_init!(Foo { 52 | | a: Bar, 53 | | b: init, 54 | | c: Bar, 55 | | d: Bar, 56 | | }) | | ^ | | | | |______expected `Bar`, found type parameter `impl PinInit<Bar>` | arguments to this function are incorrect | = note: expected struct `Bar` found type parameter `impl PinInit<Bar>` note: function defined here --> $RUSTLIB/src/rust/library/core/src/ptr/mod.rs:1398:21 | 1398 | pub const unsafe fn write<T>(dst: *mut T, src: T) { | ^^^^^ = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `pin_init` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 1 previous error ``` Gets a slight improvement: ```text error[E0308]: mismatched types --> samples/rust/rust_minimal.rs:53:9 | 50 | fn foo(init: impl PinInit<Bar>) -> impl PinInit<Foo> { | ----------------- found this type parameter 51 | / pin_init!(Foo { 52 | | a: Bar, 53 | | b: init, | | ^ expected `Bar`, found type parameter `impl PinInit<Bar>` 54 | | c: Bar, 55 | | d: Bar, 56 | | }) | |______- arguments to this function are incorrect | = note: expected struct `Bar` found type parameter `impl PinInit<Bar>` note: function defined here --> $RUSTLIB/src/rust/library/core/src/ptr/mod.rs:1398:21 | 1398 | pub const unsafe fn write<T>(dst: *mut T, src: T) { | ^^^^^ error: aborting due to 1 previous error ``` ##### When specifying the wrong initializer type (a pinned initializer when a normal one was expected) ```rust pin_init!(Foo { a: Bar, b <- init, // ^^^^ this has the wrong type c: Bar, d: Bar, }) ``` ```text error[E0277]: the trait bound `impl PinInit<Bar>: Init<Bar, _>` is not satisfied --> samples/rust/rust_minimal.rs:51:5 | 51 | / pin_init!(Foo { 52 | | a: Bar, 53 | | b <- init, | | - required by a bound introduced by this call 54 | | c: Bar, 55 | | d: Bar, 56 | | }) | |______^ the trait `Init<Bar, _>` is not implemented for `impl PinInit<Bar>` | = help: the trait `Init<impl PinInit<Bar>, _>` is implemented for `impl PinInit<Bar>` = help: for that trait implementation, expected `impl PinInit<Bar>`, found `Bar` note: required by a bound in `__ThePinData::b` --> samples/rust/rust_minimal.rs:40:1 | 40 | #[pin_data] | ^^^^^^^^^^^ required by this bound in `__ThePinData::b` ... 43 | b: Bar, | - required by a bound in this associated function = note: this error originates in the macro `$crate::__pin_data` which comes from the expansion of the attribute macro `pin_data` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 1 previous error ``` Does not really change: ```text error[E0277]: the trait bound `impl PinInit<Bar>: Init<Bar, _>` is not satisfied --> samples/rust/rust_minimal.rs:51:5 | 51 | / pin_init!(Foo { 52 | | a: Bar, 53 | | b <- init, | | - required by a bound introduced by this call 54 | | c: Bar, 55 | | d: Bar, 56 | | }) | |______^ the trait `Init<Bar, _>` is not implemented for `impl PinInit<Bar>` | = help: the trait `Init<impl PinInit<Bar>, _>` is implemented for `impl PinInit<Bar>` = help: for that trait implementation, expected `impl PinInit<Bar>`, found `Bar` note: required by a bound in `__ThePinData::b` --> samples/rust/rust_minimal.rs:40:1 | 40 | #[pin_data] | ^^^^^^^^^^^ required by this bound in `__ThePinData::b` ... 43 | b: Bar, | - required by a bound in this associated function = note: this error originates in the attribute macro `pin_data` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 1 previous error ``` ## Discussion Questions <!-- Anything that requires lengthy discussion/more general questions also fit here --> ### `.rsi` targets for things inside of the `kernel` crate When running `make LLVM=1 rust/kernel/workqueue.rsi` it does not work. Probably because the code is taken to be its own crate. Is this something the new buildsystem will take care of? Also I never am able to remember that the extension is `.rsi`, can we maybe change it to `.expanded.rs`? since then editors are also enabling rust syntax immediately. ### `static mut` vs `static Opaque` I'm just curious about the difference of the following two declarations: ```rust= extern "C" { pub static __this_module: Opaque<bindings::module>; } ``` ```rust= extern "C" { pub static mut __this_module: bindings::module; } ``` and whether it would make any difference in our usage: https://lore.kernel.org/rust-for-linux/bcbc1c4c-1e26-4afc-86ad-5cefa3dd26e0@proton.me/ (btw, `addr_of_mut!` requires unstable features) Gary: What unstable feature is needed? Alice: Those should be the same. Boqun: @nbdd0121 const_mut_refs. ### LTO Unconditionally call into llvm-link ## Miscellaneous <!-- stuff that does not fit into other categories -->