| Message ID | 20231017113014.3492773-4-fujita.tomonori@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Rust abstractions for network PHY drivers | expand |
FUJITA Tomonori <fujita.tomonori@gmail.com> writes: > From: Miguel Ojeda <ojeda@kernel.org> > > error[E0005]: refutable pattern in function argument > --> rust/bindings/bindings_enum_check.rs:29:6 > | > 29 | (phy_state::PHY_DOWN > | ______^ > 30 | | | phy_state::PHY_READY > 31 | | | phy_state::PHY_HALTED > 32 | | | phy_state::PHY_ERROR > ... | > 35 | | | phy_state::PHY_NOLINK > 36 | | | phy_state::PHY_CABLETEST): phy_state, > | |______________________________^ pattern `phy_state::PHY_NEW` not covered > | > note: `phy_state` defined here > --> rust/bindings/bindings_generated_enum_check.rs:60739:10 > | > 60739 | pub enum phy_state { > | ^^^^^^^^^ > ... > 60745 | PHY_NEW = 5, > | ------- not covered > = note: the matched value is of type `phy_state` > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> This patch does not build for me. Do I need to do something to make it work? BR Andreas
On Fri, Oct 20, 2023 at 1:42 PM Andreas Hindborg (Samsung) <nmi@metaspace.dk> wrote: > > This patch does not build for me. Do I need to do something to make it > work? Please change: - --crate-name enum_check $(obj)/bindings/bindings_enum_check.rs + --crate-name enum_check $(srctree)/$(src)/bindings/bindings_enum_check.rs Cheers, Miguel
On Fri, Oct 20, 2023 at 2:34 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > Please change: > > - --crate-name enum_check $(obj)/bindings/bindings_enum_check.rs > + --crate-name enum_check > $(srctree)/$(src)/bindings/bindings_enum_check.rs ...without the email wrapping. Cheers, Miguel
On Fri, 20 Oct 2023 13:37:14 +0200 "Andreas Hindborg (Samsung)" <nmi@metaspace.dk> wrote: > > FUJITA Tomonori <fujita.tomonori@gmail.com> writes: > >> From: Miguel Ojeda <ojeda@kernel.org> >> >> error[E0005]: refutable pattern in function argument >> --> rust/bindings/bindings_enum_check.rs:29:6 >> | >> 29 | (phy_state::PHY_DOWN >> | ______^ >> 30 | | | phy_state::PHY_READY >> 31 | | | phy_state::PHY_HALTED >> 32 | | | phy_state::PHY_ERROR >> ... | >> 35 | | | phy_state::PHY_NOLINK >> 36 | | | phy_state::PHY_CABLETEST): phy_state, >> | |______________________________^ pattern `phy_state::PHY_NEW` not covered >> | >> note: `phy_state` defined here >> --> rust/bindings/bindings_generated_enum_check.rs:60739:10 >> | >> 60739 | pub enum phy_state { >> | ^^^^^^^^^ >> ... >> 60745 | PHY_NEW = 5, >> | ------- not covered >> = note: the matched value is of type `phy_state` >> >> Signed-off-by: Miguel Ojeda <ojeda@kernel.org> > > This patch does not build for me. Do I need to do something to make it > work? Hmm, this works for me. Note that it depends on the first patch (the changes to binings_helper.h).
On Sat, Oct 21, 2023 at 5:51 AM FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > Hmm, this works for me. Andreas was probably using `O=`, but you were not. At least that is what I guessed yesterday and what the suggestion I gave fixes. This is also why sending unfinished work by someone else is not the best idea. I would also have marked that patch as RFC and put it at the end perhaps, to make it clearer. By the way, the patch is missing your SoB. I would recommend using `--signoff` in Git and b4's `am`, `cherry-pick` etc. Cheers, Miguel
On Sat, 21 Oct 2023 14:05:53 +0200 Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > On Sat, Oct 21, 2023 at 5:51 AM FUJITA Tomonori > <fujita.tomonori@gmail.com> wrote: >> >> Hmm, this works for me. > > Andreas was probably using `O=`, but you were not. > > At least that is what I guessed yesterday and what the suggestion I gave fixes. I see, thanks. > This is also why sending unfinished work by someone else is not the > best idea. I would also have marked that patch as RFC and put it at > the end perhaps, to make it clearer. > > By the way, the patch is missing your SoB. I would recommend using > `--signoff` in Git and b4's `am`, `cherry-pick` etc. Incorporated the Makefile fix and added my Signed-off-by. You would like to keep WIP in the subject? = From ff5b567a8b4e4a48f8a0c7f1ac71f8ef2c2ed226 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda <ojeda@kernel.org> Date: Fri, 13 Oct 2023 12:37:58 +0200 Subject: [PATCH] WIP rust: add second `bindgen` pass for enum exhaustiveness checking error[E0005]: refutable pattern in function argument --> rust/bindings/bindings_enum_check.rs:29:6 | 29 | (phy_state::PHY_DOWN | ______^ 30 | | | phy_state::PHY_READY 31 | | | phy_state::PHY_HALTED 32 | | | phy_state::PHY_ERROR ... | 35 | | | phy_state::PHY_NOLINK 36 | | | phy_state::PHY_CABLETEST): phy_state, | |______________________________^ pattern `phy_state::PHY_NEW` not covered | note: `phy_state` defined here --> rust/bindings/bindings_generated_enum_check.rs:60739:10 | 60739 | pub enum phy_state { | ^^^^^^^^^ ... 60745 | PHY_NEW = 5, | ------- not covered = note: the matched value is of type `phy_state` Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> --- rust/.gitignore | 1 + rust/Makefile | 14 +++++++++++ rust/bindings/bindings_enum_check.rs | 36 ++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 rust/bindings/bindings_enum_check.rs diff --git a/rust/.gitignore b/rust/.gitignore index d3829ffab80b..1a76ad0d6603 100644 --- a/rust/.gitignore +++ b/rust/.gitignore @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 bindings_generated.rs +bindings_generated_enum_check.rs bindings_helpers_generated.rs doctests_kernel_generated.rs doctests_kernel_generated_kunit.c diff --git a/rust/Makefile b/rust/Makefile index a27f35f924ec..b37842120b74 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -15,6 +15,7 @@ always-$(CONFIG_RUST) += libmacros.so no-clean-files += libmacros.so always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs +always-$(CONFIG_RUST) += bindings/bindings_generated_enum_check.rs obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o always-$(CONFIG_RUST) += exports_alloc_generated.h exports_bindings_generated.h \ exports_kernel_generated.h @@ -341,6 +342,19 @@ $(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_helper.h \ $(src)/bindgen_parameters FORCE $(call if_changed_dep,bindgen) +$(obj)/bindings/bindings_generated_enum_check.rs: private bindgen_target_flags = \ + $(shell grep -v '^#\|^$$' $(srctree)/$(src)/bindgen_parameters) \ + --default-enum-style rust +$(obj)/bindings/bindings_generated_enum_check.rs: private bindgen_target_extra = ; \ + OBJTREE=$(abspath $(objtree)) $(RUSTC_OR_CLIPPY) $(rust_flags) $(rustc_target_flags) \ + --crate-type rlib -L$(objtree)/$(obj) \ + --emit=dep-info=$(obj)/bindings/.bindings_enum_check.rs.d \ + --emit=metadata=$(obj)/bindings/libbindings_enum_check.rmeta \ + --crate-name enum_check $(srctree)/$(src)/bindings/bindings_enum_check.rs +$(obj)/bindings/bindings_generated_enum_check.rs: $(src)/bindings/bindings_helper.h \ + $(src)/bindings/bindings_enum_check.rs $(src)/bindgen_parameters FORCE + $(call if_changed_dep,bindgen) + $(obj)/uapi/uapi_generated.rs: private bindgen_target_flags = \ $(shell grep -Ev '^#|^$$' $(srctree)/$(src)/bindgen_parameters) $(obj)/uapi/uapi_generated.rs: $(src)/uapi/uapi_helper.h \ diff --git a/rust/bindings/bindings_enum_check.rs b/rust/bindings/bindings_enum_check.rs new file mode 100644 index 000000000000..eef7e9ca3c54 --- /dev/null +++ b/rust/bindings/bindings_enum_check.rs @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Bindings' enum exhaustiveness check. +//! +//! Eventually, this should be replaced by a safe version of `--rustified-enum`, see +//! https://github.com/rust-lang/rust-bindgen/issues/2646. + +#![no_std] +#![allow( + clippy::all, + dead_code, + missing_docs, + non_camel_case_types, + non_upper_case_globals, + non_snake_case, + improper_ctypes, + unreachable_pub, + unsafe_op_in_unsafe_fn +)] + +include!(concat!( + env!("OBJTREE"), + "/rust/bindings/bindings_generated_enum_check.rs" +)); + +fn check_phy_state( + (phy_state::PHY_DOWN + | phy_state::PHY_READY + | phy_state::PHY_HALTED + | phy_state::PHY_ERROR + | phy_state::PHY_UP + | phy_state::PHY_RUNNING + | phy_state::PHY_NOLINK + | phy_state::PHY_CABLETEST): phy_state, +) { +}
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes: > On Fri, Oct 20, 2023 at 1:42 PM Andreas Hindborg (Samsung) > <nmi@metaspace.dk> wrote: >> >> This patch does not build for me. Do I need to do something to make it >> work? > > Please change: > > - --crate-name enum_check $(obj)/bindings/bindings_enum_check.rs > + --crate-name enum_check > $(srctree)/$(src)/bindings/bindings_enum_check.rs This works, thanks
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes: > On Sat, Oct 21, 2023 at 5:51 AM FUJITA Tomonori > <fujita.tomonori@gmail.com> wrote: >> >> Hmm, this works for me. > > Andreas was probably using `O=`, but you were not. > > At least that is what I guessed yesterday and what the suggestion I gave fixes. Yes, out of tree build
diff --git a/rust/.gitignore b/rust/.gitignore index d3829ffab80b..1a76ad0d6603 100644 --- a/rust/.gitignore +++ b/rust/.gitignore @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 bindings_generated.rs +bindings_generated_enum_check.rs bindings_helpers_generated.rs doctests_kernel_generated.rs doctests_kernel_generated_kunit.c diff --git a/rust/Makefile b/rust/Makefile index 87958e864be0..4a1c7a48dfad 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -15,6 +15,7 @@ always-$(CONFIG_RUST) += libmacros.so no-clean-files += libmacros.so always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs +always-$(CONFIG_RUST) += bindings/bindings_generated_enum_check.rs obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o always-$(CONFIG_RUST) += exports_alloc_generated.h exports_bindings_generated.h \ exports_kernel_generated.h @@ -341,6 +342,19 @@ $(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_helper.h \ $(src)/bindgen_parameters FORCE $(call if_changed_dep,bindgen) +$(obj)/bindings/bindings_generated_enum_check.rs: private bindgen_target_flags = \ + $(shell grep -v '^#\|^$$' $(srctree)/$(src)/bindgen_parameters) \ + --default-enum-style rust +$(obj)/bindings/bindings_generated_enum_check.rs: private bindgen_target_extra = ; \ + OBJTREE=$(abspath $(objtree)) $(RUSTC_OR_CLIPPY) $(rust_flags) $(rustc_target_flags) \ + --crate-type rlib -L$(objtree)/$(obj) \ + --emit=dep-info=$(obj)/bindings/.bindings_enum_check.rs.d \ + --emit=metadata=$(obj)/bindings/libbindings_enum_check.rmeta \ + --crate-name enum_check $(obj)/bindings/bindings_enum_check.rs +$(obj)/bindings/bindings_generated_enum_check.rs: $(src)/bindings/bindings_helper.h \ + $(src)/bindings/bindings_enum_check.rs $(src)/bindgen_parameters FORCE + $(call if_changed_dep,bindgen) + $(obj)/uapi/uapi_generated.rs: private bindgen_target_flags = \ $(shell grep -v '^#\|^$$' $(srctree)/$(src)/bindgen_parameters) $(obj)/uapi/uapi_generated.rs: $(src)/uapi/uapi_helper.h \ diff --git a/rust/bindings/bindings_enum_check.rs b/rust/bindings/bindings_enum_check.rs new file mode 100644 index 000000000000..eef7e9ca3c54 --- /dev/null +++ b/rust/bindings/bindings_enum_check.rs @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Bindings' enum exhaustiveness check. +//! +//! Eventually, this should be replaced by a safe version of `--rustified-enum`, see +//! https://github.com/rust-lang/rust-bindgen/issues/2646. + +#![no_std] +#![allow( + clippy::all, + dead_code, + missing_docs, + non_camel_case_types, + non_upper_case_globals, + non_snake_case, + improper_ctypes, + unreachable_pub, + unsafe_op_in_unsafe_fn +)] + +include!(concat!( + env!("OBJTREE"), + "/rust/bindings/bindings_generated_enum_check.rs" +)); + +fn check_phy_state( + (phy_state::PHY_DOWN + | phy_state::PHY_READY + | phy_state::PHY_HALTED + | phy_state::PHY_ERROR + | phy_state::PHY_UP + | phy_state::PHY_RUNNING + | phy_state::PHY_NOLINK + | phy_state::PHY_CABLETEST): phy_state, +) { +}