Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New syn #31

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

New syn #31

wants to merge 18 commits into from

Conversation

y86-dev
Copy link
Member

@y86-dev y86-dev commented Mar 4, 2025

No description provided.

y86-dev and others added 16 commits March 1, 2025 22:13
Adds the `syn` crate as a dependency in order to rewrite the proc macros
using it.

Signed-off-by: Benno Lossin <[email protected]>
Implement the `Zeroable` derive macro using syn to simplify parsing by
not going through an additional declarative macro.

The syn version is only enabled in the user-space version and disabled
in the kernel until syn becomes available there.

Signed-off-by: Benno Lossin <[email protected]>
Implement the `pinned_drop` attribute macro using syn to simplify
parsing by not going through an additional declarative macro.
This not only simplifies the code by a lot, increasing maintainability
and making it easier to implement new features. But also improves the
user experience by improving the error messages one gets when giving
incorrect inputs to the macro.
For example in this piece of code, there is a `drop` function missing:

    use pin_init::*;

    #[pin_data(PinnedDrop)]
    struct Foo {}

    #[pinned_drop]
    impl PinnedDrop for Foo {}

But this error is communicated very poorly in the declarative macro
version:

    error: no rules expected `)`
      |
    6 | #[pinned_drop]
      | ^^^^^^^^^^^^^^ no rules expected this token in macro call
      |
    note: while trying to match keyword `fn`
     --> src/macros.rs
      |
      |             fn drop($($sig:tt)*) {
      |             ^^
      = note: this error originates in the attribute macro `pinned_drop` (in Nightly builds, run with -Z macro-backtrace for more info)

    error[E0277]: the trait bound `Foo: PinnedDrop` is not satisfied
      |
    3 | #[pin_data(PinnedDrop)]
      | ^^^^^^^^^^^^^^^^^^^^^^^
      | |
      | the trait `PinnedDrop` is not implemented for `Foo`
      | required by a bound introduced by this call
      |
      = 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)

The syn version is much more concise and right to the point:

    error[E0046]: not all trait items implemented, missing: `drop`
      |
    7 | impl PinnedDrop for Foo {}
      | ^^^^^^^^^^^^^^^^^^^^^^^ missing `drop` in implementation
      |
      = help: implement the missing item: `fn drop(self: Pin<&mut Self>, _: OnlyCallFromDrop) { todo!() }`

Another example is the following:

    use pin_init::*;
    use std::pin::Pin;

    #[pin_data(PinnedDrop)]
    struct Foo {}

    #[pinned_drop]
    impl PinnedDrop for Foo {
        fn drop(self: Pin<&mut Self>) {}

        const BAZ: usize = 0;
    }

It produces this error in the declarative macro version:

    error: no rules expected keyword `const`
       |
    10 |     const BAZ: usize = 0;
       |     ^^^^^ no rules expected this token in macro call
       |
    note: while trying to match `)`
      --> src/macros.rs
       |
       |         ),
       |         ^

    error[E0277]: the trait bound `Foo: PinnedDrop` is not satisfied
      |
    3 | #[pin_data(PinnedDrop)]
      | ^^^^^^^^^^^^^^^^^^^^^^^
      | |
      | the trait `PinnedDrop` is not implemented for `Foo`
      | required by a bound introduced by this call
      |
      = 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)

In the syn version, we get instead:

    error[E0438]: const `BAZ` is not a member of trait `pinned_init::PinnedDrop`
       |
    11 |     const BAZ: usize = 0;
       |     ^^^^^^^^^^^^^^^^^^^^^ not a member of trait `pinned_init::PinnedDrop`

The syn version is only enabled in the user-space version and disabled
in the kernel until syn becomes available there.

Signed-off-by: Benno Lossin <[email protected]>
Implement the `pin_data` attribute macro using syn to simplify parsing
by not going through an additional declarative macro.
This not only simplifies the code by a lot, increasing maintainability
and making it easier to implement new features. But also improves the
user experience by improving the error messages one gets when giving
incorrect inputs to the macro.
For example, annotating a function with `pin_data` is not allowed:

    use pin_init::*;

    #[pin_data]
    fn foo() {}

This results in the following rather unwieldy error with the declarative
version:

    error: no rules expected keyword `fn`
      |
    4 | fn foo() {}
      | ^^ no rules expected this token in macro call
      |
    note: while trying to match keyword `struct`
     --> src/macros.rs
      |
      |             $vis:vis struct $name:ident
      |                      ^^^^^^

    error: Could not locate type name.
      |
    3 | #[pin_data]
      | ^^^^^^^^^^^
      |
      = note: this error originates in the attribute macro `pin_data` (in Nightly builds, run with -Z macro-backtrace for more info)

The syn version gives this very concise error:

    error: expected `struct`
      |
    4 | fn foo() {}
      | ^^

The syn version is only enabled in the user-space version and disabled
in the kernel until syn becomes available there.

Signed-off-by: Benno Lossin <[email protected]>
`#[pin_data]` uses some auxiliary traits to ensure that a user does not
implement `Drop` for the annotated struct, as that is unsound and can
lead to UB. However, if the struct that is annotated is `!Sized`, the
current bounds do not work, because `Sized` is an implicit bound for
generics.

This is *not* a soundness hole of pin-init, as it currently is
impossible to construct an unsized value using pin-init.

Signed-off-by: Benno Lossin <[email protected]>
The next commit will name the pin-init crate from proc macros via
`::pin_init`. For this to work within tests of the pin-init crate
itself, it needs to be able to refer to itself via that name. Thus add
the required code for the name to be available.

Signed-off-by: Benno Lossin <[email protected]>
@y86-dev y86-dev force-pushed the new-syn branch 3 times, most recently from 87b3181 to 9ce57fb Compare March 4, 2025 22:38
Implement the `[try_][pin_]init!` derive macro using syn to simplify
parsing by not going through an additional declarative macro.
This not only simplifies the code by a lot, increasing maintainability
and making it easier to implement new features. But also improves the
user experience by improving the error messages one gets when giving
incorrect inputs to the macro.
For example, placing a `,` after `..Zeroable::zeroed()` is not allowed:

    use pin_init::*;

    #[derive(Zeroable)]
    struct Foo {
        a: usize,
        b: usize,
    }

    fn main() {
        let _ = init!(Foo {
            a: 0,
            ..Zeroable::zeroed(),
        });
    }

The declarative macro produces this error:

    error: no rules expected `,`
       |
    11 |       let _ = init!(Foo {
       |  _____________^
    12 | |         a: 0,
    13 | |         ..Zeroable::zeroed(),
    14 | |     });
       | |______^ no rules expected this token in macro call
       |
    note: while trying to match `)`
      --> src/macros.rs
       |
       |         @munch_fields($(..Zeroable::zeroed())? $(,)?),
       |                                                     ^
       = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info)

    error: no rules expected `,`
       |
    11 |       let _ = init!(Foo {
       |  _____________^
    12 | |         a: 0,
    13 | |         ..Zeroable::zeroed(),
    14 | |     });
       | |______^ no rules expected this token in macro call
       |
    note: while trying to match `)`
      --> src/macros.rs
       |
       |         @munch_fields(..Zeroable::zeroed() $(,)?),
       |                                                 ^
       = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info)

The syn version reduces this error to the much more manageable:

    error: unexpected token, expected `}`
       |
    12 |         ..Zeroable::zeroed(),
       |                             ^

This reimplementation is benefiting the most from syn, as can be seen in
this example. It declares a struct with a single generic, but then
supplies two type arguments in the initializer:

    use pin_init::*;

    struct Foo<T> {
        value: T,
    }
    fn main() {
        let _ = init!(Foo::<(), ()> {
            value <- (),
        });
    }

The declarative version emits the following unreadable mess of an error
(shortened for brevity of the commit message):

    error: struct literal body without path
      |
    7 |       let _ = init!(Foo::<(), ()> {
      |  _____________^
    8 | |         value <- (),
    9 | |     });
      | |______^
      |
      = note: this error originates in the macro `$crate::__init_internal` which comes from the expansion of the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info)
    help: you might have forgotten to add the struct literal inside the block
     --> src/macros.rs
      |
      ~                 ::core::ptr::write($slot, $t { SomeStruct {
      |9                    $($acc)*
      ~                 } });
      |

<...40 lines skipped...>

    error[E0061]: this function takes 2 arguments but 3 arguments were supplied
      |
    7 |       let _ = init!(Foo::<(), ()> {
      |  _____________^
    8 | |         value <- (),
    9 | |     });
      | |______^ unexpected argument #3
      |
    note: function defined here
     --> $RUST/core/src/ptr/mod.rs
      |
      | 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 `init` (in Nightly builds, run with -Z macro-backtrace for more info)

This error delightfully reduces to the simple and clear message:

    error[E0107]: struct takes 1 generic argument but 2 generic arguments were supplied
      |
    7 |     let _ = init!(Foo::<(), ()> {
      |                   ^^^     ---- help: remove the unnecessary generic argument
      |                   |
      |                   expected 1 generic argument
      |
    note: struct defined here, with 1 generic parameter: `T`
     --> tests/ui/compile-fail/init/wrong_generics2.rs:3:8
      |
    3 | struct Foo<T> {
      |        ^^^ -

The syn version is only enabled in the user-space version and disabled
in the kernel until syn becomes available there.

Signed-off-by: Benno Lossin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant