Macro syn rewrite #1073
Draft
Macro syn rewrite #1073
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
y86-dev
force-pushed
the
macro-syn-rewrite
branch
2 times, most recently
from
bc10cb7 to
3b8d505
Compare
Host programs should be (and currently are) warning-free. Apply `CONFIG_WERROR` (i.e. `-Dwarnings`) to them as well, so that they are kept that way. Signed-off-by: Miguel Ojeda <[email protected]>
Proc macros run in the host, so apply those flags for them instead of
just the common ones.
In practice, this currently adds:
-O -Cstrip=debuginfo -Zallow-features=
and, if enabled, `-Dwarnings` too (i.e. `CONFIG_WERROR`) -- the `macros`
crate should be (and currently is) warning-free, so applying this to it
ensures it is kept that way.
Signed-off-by: Miguel Ojeda <[email protected]>
There are two kinds of configuration options (`cfg`s): names and key-value pairs. So far, we only needed to support the former for the `rust-analyzer` target. Since we are introducing `syn` (and its dependencies) in later patches, which require passing `feature` `cfg`s, support key-value `cfg`s too in the `rust-analyzer` target. Signed-off-by: Miguel Ojeda <[email protected]>
This is a subset of the Rust `proc-macro2` crate,
version 1.0.79, licensed under "Apache-2.0 OR MIT", from:
https://github.com/dtolnay/proc-macro2/raw/1.0.79/src
The files are copied as-is, with no modifications whatsoever
(not even adding the SPDX identifiers).
For copyright details, please see:
https://github.com/dtolnay/proc-macro2/blob/1.0.79/README.md#license
https://github.com/dtolnay/proc-macro2/blob/1.0.79/LICENSE-APACHE
https://github.com/dtolnay/proc-macro2/blob/1.0.79/LICENSE-MIT
The next two patches modify these files as needed for use within
the kernel. This patch split allows reviewers to double-check
the import and to clearly see the differences introduced.
The following script may be used to verify the contents:
for path in $(cd rust/proc-macro2/ && find . -type f -name '*.rs'); do
curl --silent --show-error --location \
https://github.com/dtolnay/proc-macro2/raw/1.0.79/src/$path \
| diff --unified rust/proc-macro2/$path - && echo $path: OK
done
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
The `proc-macro2` crate depends on the `unicode-ident` crate to determine whether characters have the XID_Start or XID_Continue properties according to Unicode Standard Annex Rust-for-Linux#31. However, we only need ASCII identifiers in the kernel, thus we can simplify the check and remove completely that dependency. Signed-off-by: Miguel Ojeda <[email protected]>
This is a subset of the Rust `quote` crate,
version 1.0.35, licensed under "Apache-2.0 OR MIT", from:
https://github.com/dtolnay/quote/raw/1.0.35/src
The files are copied as-is, with no modifications whatsoever
(not even adding the SPDX identifiers).
For copyright details, please see:
https://github.com/dtolnay/quote/blob/1.0.35/README.md#license
https://github.com/dtolnay/quote/blob/1.0.35/LICENSE-APACHE
https://github.com/dtolnay/quote/blob/1.0.35/LICENSE-MIT
The next patch modifies these files as needed for use within
the kernel. This patch split allows reviewers to double-check
the import and to clearly see the differences introduced.
The following script may be used to verify the contents:
for path in $(cd rust/quote/ && find . -type f -name '*.rs'); do
curl --silent --show-error --location \
https://github.com/dtolnay/quote/raw/1.0.35/src/$path \
| diff --unified rust/quote/$path - && echo $path: OK
done
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
This is a subset of the Rust `syn` crate,
version 2.0.58, licensed under "Apache-2.0 OR MIT", from:
https://github.com/dtolnay/syn/raw/2.0.58/src
The files are copied as-is, with no modifications whatsoever
(not even adding the SPDX identifiers).
For copyright details, please see:
https://github.com/dtolnay/syn/blob/2.0.58/README.md#license
https://github.com/dtolnay/syn/blob/2.0.58/LICENSE-APACHE
https://github.com/dtolnay/syn/blob/2.0.58/LICENSE-MIT
The next two patches modify these files as needed for use within
the kernel. This patch split allows reviewers to double-check
the import and to clearly see the differences introduced.
The following script may be used to verify the contents:
for path in $(cd rust/syn/ && find . -type f -name '*.rs'); do
curl --silent --show-error --location \
https://github.com/dtolnay/syn/raw/2.0.58/src/$path \
| diff --unified rust/syn/$path - && echo $path: OK
done
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
The `syn` crate depends on the `unicode-ident` crate to determine whether characters have the XID_Start or XID_Continue properties according to Unicode Standard Annex Rust-for-Linux#31. However, we only need ASCII identifiers in the kernel, thus we can simplify the check and remove completely that dependency. Signed-off-by: Miguel Ojeda <[email protected]>
With all the new files in place from the new crates, this patch adds support for them in the build system so that the kernel's `macros` crate can use all their facilities. In particular, the `syn` crate is fully enabled, i.e. all its optional dependencies are enabled. Signed-off-by: Miguel Ojeda <[email protected]>
The `module!` macro creates glue code that are called by C to initialize the Rust modules using the `Module::init` function. Part of this glue code are the local functions `__init` and `__exit` that are used to initialize/destroy the Rust module. These functions are safe and also visible to the Rust mod in which the `module!` macro is invoked. This means that they can be called by other safe Rust code. But since they contain `unsafe` blocks that rely on only being called at the right time, this is a soundness issue. Wrap these generated functions inside of two private modules, this guarantees that the public functions cannot be called from the outside. Make the safe functions `unsafe` and add SAFETY comments. Cc: [email protected] Reported-by: Björn Roy Baron <[email protected]> Closes: Rust-for-Linux#629 Fixes: 1fbde52 ("rust: add `macros` crate") Signed-off-by: Benno Lossin <[email protected]> Reviewed-by: Wedson Almeida Filho <[email protected]> Link: https://lore.kernel.org/r/[email protected] [ Capitalized comments, avoided newline in non-list SAFETY comments and reworded to add Reported-by and newline. ] Signed-off-by: Miguel Ojeda <[email protected]>
We now have access to the `quote` crate providing the `quote!` macro that does the same (and more) as our own `quote!` macro. Thus replace our own version with the one from the crate. Since the `quote` crate uses the `proc-macro2` library, we also use that in our macros instead of `proc-macro`. Signed-off-by: Benno Lossin <[email protected]>
Now that `syn` and `quote` are available in the Kernel use them for the
`#[vtable]` proc-macro attribute instead of the current string based
proc-macro approach.
Proc-macros written using `syn` are a lot more readable and thus easier
to maintain than proc-macros written using string manipulation. An
additional advantage of `syn` is the improved error reporting, here is
an example:
#[vtable]
const _: () = ();
Prior to this patch, the error reads:
error: custom attribute panicked
--> samples/rust/rust_minimal.rs:40:1
|
40 | #[vtable]
| ^^^^^^^^^
|
= help: message: #[vtable] attribute should only be applied to trait or impl block
error: aborting due to 1 previous error
After this patch, the error reads:
error: `#[vtable]` expects a `trait` or `impl`.
--> samples/rust/rust_minimal.rs:41:1
|
41 | const _: () = ();
| ^^^^^^^^^^^^^^^^^
error: aborting due to 1 previous error
Signed-off-by: Benno Lossin <[email protected]>
Now that `syn` and `quote` are available in the Kernel use them for the
`module!` proc-macro instead of the current string based proc-macro
approach.
Proc-macros written using `syn` are a lot more readable and thus easier
to maintain than proc-macros written using string manipulation. An
additional advantage of `syn` is the improved error reporting, here is
an example:
module! {
type: RustMinimal,
name: "rust_minimal",
author: "Rust for Linux Contributors",
description: "Rust minimal sample",
// we forgot to add a `license` field.
}
Prior to this patch, the error reads:
error: proc macro panicked
--> samples/rust/rust_minimal.rs:7:1
|
7 | / module! {
8 | | type: RustMinimal,
9 | | name: "rust_minimal",
10 | | author: "Rust for Linux Contributors",
11 | | description: "Rust minimal sample",
12 | | // license: "GPL",
13 | | }
| |_^
|
= help: message: Missing required key "license".
error[E0425]: cannot find value `__LOG_PREFIX` in the crate root
--> samples/rust/rust_minimal.rs:21:9
|
21 | pr_info!("Rust minimal sample (init)\n");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in the crate root
|
= note: this error originates in the macro `$crate::print_macro` which comes from the expansion of the macro `pr_info` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0425]: cannot find value `__LOG_PREFIX` in the crate root
--> samples/rust/rust_minimal.rs:22:9
|
22 | pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in the crate root
|
= note: this error originates in the macro `$crate::print_macro` which comes from the expansion of the macro `pr_info` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0425]: cannot find value `__LOG_PREFIX` in the crate root
--> samples/rust/rust_minimal.rs:35:9
|
35 | pr_info!("My numbers are {:?}\n", self.numbers);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in the crate root
|
= note: this error originates in the macro `$crate::print_macro` which comes from the expansion of the macro `pr_info` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0425]: cannot find value `__LOG_PREFIX` in the crate root
--> samples/rust/rust_minimal.rs:36:9
|
36 | pr_info!("Rust minimal sample (exit)\n");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in the crate root
|
= note: this error originates in the macro `$crate::print_macro` which comes from the expansion of the macro `pr_info` (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to 5 previous errors
After this patch, the error reads:
error: Missing required key "license".
--> samples/rust/rust_minimal.rs:8:5
|
8 | / type: RustMinimal,
9 | | name: "rust_minimal",
10 | | author: "Rust for Linux Contributors",
11 | | description: "Rust minimal sample",
| |_______________________________________^
error: aborting due to 1 previous error
Similar improvements in error quality apply to getting the order of the
fields wrong and giving non-ascii strings as values for fields where it
should be ascii-only.
Signed-off-by: Benno Lossin <[email protected]>
Now that `syn` and `quote` are available in the Kernel use them for the
`Zeroable` derive macro instead of the current hybrid
proc-macro/declarative approach.
Proc-macros written using `syn` are a lot more readable and thus easier
to maintain than convoluted declarative macros. An additional advantage
of `syn` is the improved error reporting, here is an example:
#[derive(Zeroable)]
enum Foo {
A,
B
}
Prior to this patch, the error reads:
error: no rules expected the token `enum`
--> samples/rust/rust_minimal.rs:41:1
|
41 | enum Foo {
| ^^^^ no rules expected this token in macro call
|
note: while trying to match `struct`
--> rust/kernel/init/macros.rs:1373:22
|
1373 | $vis:vis struct $name:ident
| ^^^^^^
error: aborting due to 1 previous error
After this patch, the error reads:
error: `Zeroable` can only be derived for structs.
--> samples/rust/rust_minimal.rs:41:1
|
41 | / enum Foo {
42 | | A,
43 | | B
44 | | }
| |_^
error: aborting due to 1 previous error
Signed-off-by: Benno Lossin <[email protected]>
Now that `syn` and `quote` are available in the Kernel use them for the
`#[pin_data]` proc-macro attribute instead of the current hybrid
proc-macro/declarative approach.
Proc-macros written using `syn` are a lot more readable and thus easier
to maintain than convoluted declarative macros. An additional advantage
of `syn` is the improved error reporting, here is an example:
#[pin_data]
struct Foo {
x
}
Prior to this patch, the error reads:
error: expected `:`, found `}`
--> samples/rust/rust_minimal.rs:43:1
|
41 | struct Foo {
| --- while parsing this struct
42 | x
| - expected `:`
43 | }
| ^ unexpected token
error: recursion limit reached while expanding `$crate::__pin_data!`
--> samples/rust/rust_minimal.rs:40:1
|
40 | #[pin_data]
| ^^^^^^^^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`rust_minimal`)
= 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 2 previous errors
After this patch, the error reads:
error: expected `:`, found `}`
--> samples/rust/rust_minimal.rs:43:1
|
41 | struct Foo {
| --- while parsing this struct
42 | x
| - expected `:`
43 | }
| ^ unexpected token
error: aborting due to 1 previous error
Signed-off-by: Benno Lossin <[email protected]>
Now that `syn` and `quote` are available in the Kernel use them for the
`#[pinned_drop]` proc-macro attribute instead of the current hybrid
proc-macro/declarative approach.
Proc-macros written using `syn` are a lot more readable and thus easier
to maintain than convoluted declarative macros. An additional advantage
of `syn` is the improved error reporting, here is an example:
#[pin_data(PinnedDrop)]
struct Foo {}
#[pinned_drop]
impl PinnedDrop for Foo {}
Prior to this patch, the error reads:
error: no rules expected the token `)`
--> samples/rust/rust_minimal.rs:43:1
|
43 | #[pinned_drop]
| ^^^^^^^^^^^^^^ no rules expected this token in macro call
|
note: while trying to match `fn`
--> rust/kernel/init/macros.rs:511:13
|
511 | 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
--> samples/rust/rust_minimal.rs:40:1
|
40 | #[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)
error: aborting due to 2 previous errors
After this patch, the error reads:
error: Expected exactly one function in the `PinnedDrop` impl.
--> samples/rust/rust_minimal.rs:44:25
|
44 | impl PinnedDrop for Foo {}
| ^^
error: aborting due to 1 previous error
Signed-off-by: Benno Lossin <[email protected]>
Now that `syn` and `quote` are available in the Kernel use them for the
`__internal_init!` macro instead of the current hybrid
proc-macro/declarative approach.
Note that the new macro is called `primitive_init!`.
Proc-macros written using `syn` are a lot more readable and thus easier
to maintain than convoluted declarative macros. An additional advantage
of `syn` is the improved error reporting, here is an example:
When adding an additional `,` at the end of `..Zeroable::zeroed()`
pin_init!(Foo {
a: Bar,
b <- Bar,
c: Bar,
d: Bar,
..Zeroable::zeroed(),
// ^ this is the culprit!
})
Prior to this patch, the error reads:
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
After this patch, the error reads:
error: unexpected token
--> samples/rust/rust_minimal.rs:56:29
|
56 | ..Zeroable::zeroed(),
| ^
error: aborting due to 1 previous error
Signed-off-by: Benno Lossin <[email protected]>
Only one helper function (`expect_punct`) is used by our proc-macros. So remove all others and move `expect_punct` into concat_idents.rs. Signed-off-by: Benno Lossin <[email protected]>
Since all initialization macros have been rewritten using `syn` and `quote`, the `macros` module does no longer contain any code. The only thing that is left is documentation that explains the expansion of the initialization macros. This can now be directly observed in the source code for the macros (found under `rust/macros`). Since the proc macro code is much more readable than the declarative macro style. Thus remove the expanded code. Signed-off-by: Benno Lossin <[email protected]>
y86-dev
force-pushed
the
macro-syn-rewrite
branch
from
3b8d505 to
45d057b
Compare
fbq
reviewed
Apr 10, 2024
There was a problem hiding this comment.
Does it make sense to make it as a feature for upstream syn?
|
Another Q out of curiosity: have you measured the affect of build time? |
|
A very crude measurement:
If you time it as well, then maybe I can write about the increase in the cover letter :) |
Good point, here is the result from my machine: (configured with
|
ojeda
force-pushed
the
rust-next
branch
3 times, most recently
from
c9b5ce6 to
ce1c54f
Compare
ojeda
force-pushed
the
rust-next
branch
3 times, most recently
from
9ee7197 to
6ce162a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.