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.

Sign up for GitHub

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

rust: add proc-macro2, quote and syn support #910

Open
wants to merge 9 commits into
base: rust
Choose a base branch
from
Open

rust: add proc-macro2, quote and syn support #910

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
321a922
rust: proc-macro2: import crate
ojeda Oct 9, 2022
1a94602
rust: proc-macro2: add SPDX License Identifiers
ojeda Oct 9, 2022
51c24d4
rust: proc-macro2: remove `unicode_ident` dependency
ojeda Oct 9, 2022
725fad1
rust: quote: import crate
ojeda Oct 9, 2022
2882929
rust: quote: add SPDX License Identifiers
ojeda Oct 9, 2022
849310c
rust: syn: import crate
ojeda Oct 9, 2022
2a23ea9
rust: syn: add SPDX License Identifiers
ojeda Oct 9, 2022
4232e45
rust: syn: remove `unicode-ident` dependency
ojeda Oct 9, 2022
ab66e44
rust: Kbuild: enable `proc-macro2`, `quote` and `syn`
ojeda Oct 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1834,8 +1834,14 @@ PHONY += rustfmt rustfmtcheck
rustfmt:
$(Q)find $(abs_srctree) -type f -name '*.rs' \
-o -path $(abs_srctree)/rust/alloc -prune \
-o -path $(abs_srctree)/rust/proc-macro2 -prune \
-o -path $(abs_srctree)/rust/quote -prune \
-o -path $(abs_srctree)/rust/syn -prune \
-o -path $(abs_objtree)/rust/test -prune \
| grep -Fv $(abs_srctree)/rust/alloc \
| grep -Fv $(abs_srctree)/rust/proc-macro2 \
| grep -Fv $(abs_srctree)/rust/quote \
| grep -Fv $(abs_srctree)/rust/syn \
| grep -Fv $(abs_objtree)/rust/test \
| grep -Fv generated \
| xargs $(RUSTFMT) $(rustfmt_flags)
Expand Down
1 change: 1 addition & 0 deletions rust/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0

*.rlib
target.json
bindings_generated.rs
bindings_helpers_generated.rs
Expand Down
109 changes: 101 additions & 8 deletions rust/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ always-$(CONFIG_RUST) += exports_core_generated.h
obj-$(CONFIG_RUST) += helpers.o
CFLAGS_REMOVE_helpers.o = -Wmissing-prototypes -Wmissing-declarations

always-$(CONFIG_RUST) += libproc_macro2.rlib libquote.rlib libsyn.rlib
always-$(CONFIG_RUST) += libmacros.so
no-clean-files += libmacros.so

Expand Down Expand Up @@ -59,11 +60,56 @@ alloc-cfgs = \
--cfg no_rc \
--cfg no_sync

proc_macro2-skip_flags := \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While working on kproc-macros https://github.com/vincenzopalazzo/kproc-macros and see that prob_macros2 can be optional in projects like Linux Kernel (see https://github.com/dtolnay/syn/blob/master/Cargo.toml#L39)

I think we do not need to include proc_macro2 because it is used just for projects that import syn and quote with cargo.

P.: Well I should not say that if I want kproc-macros in right? 😸

Copy link
Member

@bjorn3 bjorn3 Nov 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/dtolnay/syn/blob/master/Cargo.toml#L39 doesn't say that proc-macro2 is optional. It says that the default features are disabled for it. Or did I misunderstand what you meant to say?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh! you are right! damnt! I think I take a look at the wrong Cargo, maybe it was the https://github.com/vincenzopalazzo/kproc-macros/blob/main/kproc-parser/Cargo.toml#L13

Apologies!

--edition=2021 \
-Drust_2018_idioms \
-Dunreachable_pub \
-Dunsafe_op_in_unsafe_fn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use --cap-lints allow instead like cargo does for non-local dependencies?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still want to guarantee some level of code quality, right? Not sure what benefits this would give us.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't fix the lints anyway without diverging from upstream, which may end up hurting more than improving. And what about if rustc gets a new lint that fires on one of these crates? If lints are denied that would end up breaking the build and the only way for us to unbreak the build without diverging from upstream would be to allow the lint.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rustc is very loathe to add warnings, but one of the larger categories we do add to often enough is future incompat warnings. Those eventually upgrade to errors, given enough time, if they are that bad. So the "code quality" may be just fine today but have some subtle detail that recommends a different pattern in the future, and in that case, you will eventually find the problem when it actually becomes upgraded.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In actual practice, I expect the kernel will use only well-tested, fully-developed but slightly-active projects that have their CI run about once every kernel release, or at worst once every few. So the most stringent I would recommend is running a cron job at either rustc's 6 week or the kernel's ~10 week cadence to check for new developments in dependencies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use --cap-lints allow instead like cargo does for non-local dependencies?

Yeah, good idea, we should do this for dependencies that we have not modified (or at least very minimally for build purposes), i.e. like these, but unlike alloc for the moment.


proc_macro2-flags := \
--edition=2018 \
-Amissing_docs \
--cfg 'feature="proc-macro"' \
--cfg use_proc_macro \
--cfg wrap_proc_macro

quote-skip_flags := \
--edition=2021 \
-Drust_2018_idioms

quote-flags := \
--edition=2018 \
-Amissing_docs \
--extern proc_macro2 \
--cfg 'feature="proc-macro"'

syn-skip_flags := \
--edition=2021 \
-Drust_2018_idioms \
-Dunreachable_pub \
-Dunsafe_op_in_unsafe_fn

syn-flags := \
--edition=2018 \
-Amissing_docs \
--cfg 'feature="clone-impls"' \
--cfg 'feature="derive"' \
--cfg 'feature="extra-traits"' \
--cfg 'feature="fold"' \
--cfg 'feature="full"' \
--cfg 'feature="parsing"' \
--cfg 'feature="printing"' \
--cfg 'feature="proc-macro"' \
--cfg 'feature="quote"' \
--cfg 'feature="visit"' \
--cfg 'feature="visit-mut"'

quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
cmd_rustdoc = \
OBJTREE=$(abspath $(objtree)) \
$(RUSTDOC) $(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) \
$(rustc_target_flags) -L$(objtree)/$(obj) \
--extern quote --extern syn \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same situation here, need --extern proc_macro2 (otherwise building docs will fail).

--output $(objtree)/$(obj)/doc \
--crate-name $(subst rustdoc-,,$@) \
@$(objtree)/include/generated/rustc_cfg $<
Expand Down Expand Up @@ -124,8 +170,9 @@ rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \
quiet_cmd_rustc_test_library = RUSTC TL $<
cmd_rustc_test_library = \
OBJTREE=$(abspath $(objtree)) \
$(RUSTC) $(rust_common_flags) \
@$(objtree)/include/generated/rustc_cfg $(rustc_target_flags) \
$(RUSTC) \
$(filter-out $(skip_flags),$(rust_common_flags) $(rustc_target_flags)) \
@$(objtree)/include/generated/rustc_cfg \
--crate-type $(if $(rustc_test_library_proc),proc-macro,rlib) \
--out-dir $(objtree)/$(obj)/test --cfg testlib \
--sysroot $(objtree)/$(obj)/test/sysroot \
Expand All @@ -135,9 +182,25 @@ quiet_cmd_rustc_test_library = RUSTC TL $<
rusttestlib-build_error: $(src)/build_error.rs rusttest-prepare FORCE
$(call if_changed,rustc_test_library)

rusttestlib-macros: private rustc_target_flags = --extern proc_macro
rusttestlib-proc_macro2: private skip_flags = $(proc_macro2-skip_flags)
rusttestlib-proc_macro2: private rustc_target_flags = $(proc_macro2-flags)
rusttestlib-proc_macro2: $(src)/proc-macro2/lib.rs rusttest-prepare FORCE
$(call if_changed,rustc_test_library)

rusttestlib-quote: private skip_flags = $(quote-skip_flags)
rusttestlib-quote: private rustc_target_flags = $(quote-flags)
rusttestlib-quote: $(src)/quote/lib.rs rusttestlib-proc_macro2 FORCE
$(call if_changed,rustc_test_library)

rusttestlib-syn: private skip_flags = $(syn-skip_flags)
rusttestlib-syn: private rustc_target_flags = $(syn-flags)
rusttestlib-syn: $(src)/syn/lib.rs rusttestlib-quote FORCE
$(call if_changed,rustc_test_library)

rusttestlib-macros: private rustc_target_flags = --extern proc_macro \
--extern quote --extern syn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same situation here, need --extern proc_macro2 (otherwise buliding rusttest will fail).

rusttestlib-macros: private rustc_test_library_proc = yes
rusttestlib-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
rusttestlib-macros: $(src)/macros/lib.rs rusttestlib-syn FORCE
$(call if_changed,rustc_test_library)

rusttestlib-bindings: $(src)/bindings/lib.rs rusttest-prepare FORCE
Expand Down Expand Up @@ -236,9 +299,10 @@ quiet_cmd_rustsysroot = RUSTSYSROOT
rusttest-prepare: FORCE
$(call if_changed,rustsysroot)

rusttest-macros: private rustc_target_flags = --extern proc_macro
rusttest-macros: private rustc_target_flags = --extern proc_macro \
--extern quote --extern syn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same situation here, need --extern proc_macro2 (otherwise buliding rusttest will fail).

rusttest-macros: private rustdoc_test_target_flags = --crate-type proc-macro
rusttest-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
rusttest-macros: $(src)/macros/lib.rs rusttestlib-syn FORCE
$(call if_changed,rustc_test)
$(call if_changed,rustdoc_test)

Expand Down Expand Up @@ -355,19 +419,48 @@ $(obj)/exports_bindings_generated.h: $(obj)/bindings.o FORCE
$(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
$(call if_changed,exports)

quiet_cmd_rustc_hostlibrary = $(RUSTC_OR_CLIPPY_QUIET) H $@
cmd_rustc_hostlibrary = \
$(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
$(filter-out $(skip_flags),$(rust_common_flags) $(rustc_target_flags)) \
--emit=dep-info,link --crate-type rlib -O \
--out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \
--crate-name $(patsubst lib%.rlib,%,$(notdir $@)) $<; \
mv $(objtree)/$(obj)/$(patsubst lib%.rlib,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)

$(obj)/libproc_macro2.rlib: private skip_clippy = 1
$(obj)/libproc_macro2.rlib: private skip_flags = $(proc_macro2-skip_flags)
$(obj)/libproc_macro2.rlib: private rustc_target_flags = $(proc_macro2-flags)
$(obj)/libproc_macro2.rlib: $(src)/proc-macro2/lib.rs FORCE
$(call if_changed_dep,rustc_hostlibrary)

$(obj)/libquote.rlib: private skip_clippy = 1
$(obj)/libquote.rlib: private skip_flags = $(quote-skip_flags)
$(obj)/libquote.rlib: private rustc_target_flags = $(quote-flags)
$(obj)/libquote.rlib: $(src)/quote/lib.rs $(obj)/libproc_macro2.rlib FORCE
$(call if_changed_dep,rustc_hostlibrary)

$(obj)/libsyn.rlib: private skip_clippy = 1
$(obj)/libsyn.rlib: private skip_flags = $(syn-skip_flags)
$(obj)/libsyn.rlib: private rustc_target_flags = $(syn-flags)
$(obj)/libsyn.rlib: $(src)/syn/lib.rs $(obj)/libquote.rlib FORCE
$(call if_changed_dep,rustc_hostlibrary)

quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
cmd_rustc_procmacro = \
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
--emit=dep-info,link --extern proc_macro \
--crate-type proc-macro --out-dir $(objtree)/$(obj) \
--extern quote --extern syn --crate-type proc-macro \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need --extern proc_macro2 here, otherwise we cannot use it in macros.

--out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \
--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
mv $(objtree)/$(obj)/$(patsubst lib%.so,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)

# Procedural macros can only be used with the `rustc` that compiled it.
# Therefore, to get `libmacros.so` automatically recompiled when the compiler
# version changes, we add `core.o` as a dependency (even if it is not needed).
$(obj)/libmacros.so: $(src)/macros/lib.rs $(obj)/core.o FORCE
$(obj)/libmacros.so: $(src)/macros/lib.rs $(obj)/libsyn.rlib $(obj)/core.o FORCE
$(call if_changed_dep,rustc_procmacro)

quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L $@
Expand Down