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

Jump to bottom

Custom dbg! macros for dbg_macro lint #11303

Open
ojeda opened this issue Aug 8, 2023 · 2 comments
Open

Custom dbg! macros for dbg_macro lint #11303

ojeda opened this issue Aug 8, 2023 · 2 comments

Comments

@ojeda
Copy link
Contributor

ojeda commented Aug 8, 2023 •
edited
Loading

Back in Rust 1.60.0 we added -Wclippy::dbg_macro to the Linux kernel compilation flags, which worked great with our custom dbg! macro (essentially the std one but calling the kernel printing facilities).

However, in the next version (1.61.0), as well as the current stable (1.80.0) and the current nightly (1.83.0-nightly (bd53aa3 2024-09-02)), it does not work. Thus only the first of the following lines emit a warning:

echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run  1.60.0 clippy-driver -Wclippy::dbg_macro -
echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run  1.61.0 clippy-driver -Wclippy::dbg_macro -
echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run  1.80.0 clippy-driver -Wclippy::dbg_macro -
echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run nightly clippy-driver -Wclippy::dbg_macro -

Is the lint intended to work only with the standard library dbg macro? I imagine that is the case, given the move to diagnostic items in #7466, but it is not entirely clear from the lint description.

A workaround is to use disallowed_macros, but that does not have a specialized diagnostic message nor has the nice help: suggestion. Another is to use rustc_attrs, but I imagine that is not going to be stable.

Thus, instead, could the dbg_macro lint take a configuration with paths to dbg! macros (like disallowed_macros) or, even better, could there be an attribute that users could apply to their dbg! macro (like rustc_diagnostic_item)?

Cc @xFrednet and @Alexendoo who both seemed to work on this in the past.
@ojeda ojeda mentioned this issue Aug 8, 2023
Open
32 tasks
@Centri3
Copy link
Member

Centri3 commented Aug 8, 2023 •
edited
Loading

I'd assume it was this PR which changed it from a simple path check to a diagnostic item: #8411

The lint's purpose is only the dbg! macro, nothing more. Instead of making it work on arbitrary macros, we should probably enhance the diagnostics from disallowed_macros. Maybe we can add an option for "this macro invocation just repeats its input, so suggest to remove it entirely"? As that's what dbg! does already. We can probably also add an option to not show the "use of a disallowed macro" part and allow the user to set the message arbitrarily.

@ojeda
Copy link
Contributor Author

ojeda commented Aug 8, 2023 •
edited
Loading

If one gets that level of customization, then it would be fine. Though if we wanted it to be as flexible, we would also need to be able to customize the lint level per path (which may be desirable anyway -- filled #11307), since right now a project may put a different level to dbg_macro vs. disallowed_macros.

@ojeda ojeda mentioned this issue Aug 18, 2023
Open
37 tasks
ojeda added a commit to ojeda/linux that referenced this issue Sep 3, 2024
@ojeda
rust: replace clippy::dbg_macro with disallowed_macros
dee38ab 
Back when we used Rust 1.60.0 (before Rust was merged in the kernel),
we added `-Wclippy::dbg_macro` to the compilation flags. This worked
great with our custom `dbg!` macro (vendored from `std`, but slightly
modified to use the kernel printing facilities).

However, in the very next version, 1.61.0, it stopped working [1] since
the lint started to use a Rust diagnostic item rather than a path to find
the `dbg!` macro [1]. This behavior remains until the current nightly
(1.83.0).

Therefore, currently, the `dbg_macro` is not doing anything, which
explains why we can invoke `dbg!` in samples/rust/rust_print.rs`, as well
as why changing the `#[allow()]`s to `#[expect()]`s in `std_vendor.rs`
doctests does not work since they are not fulfilled.

One possible workaround is using `rustc_attrs` like the standard library
does. However, this is intended to be internal, and we just started
supporting several Rust compiler versions, so it is best to avoid it.

Therefore, instead, use `disallowed_macros`. It is a stable lint and
is more flexible (in that we can provide different macros), although
its diagnostic message(s) are not as nice as the specialized one (yet),
and does not allow to set different lint levels per macro/path [2].

In turn, this requires allowing the (intentional) `dbg!` use in the
sample, as one would have expected.

Finally, in a single case, the `allow` is fixed to be an inner attribute,
since otherwise it was not being applied.

Link: rust-lang/rust-clippy#11303 [1]
Link: rust-lang/rust-clippy#11307 [2]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to ojeda/linux that referenced this issue Sep 4, 2024
@ojeda
rust: replace clippy::dbg_macro with disallowed_macros
8c61e34 
Back when we used Rust 1.60.0 (before Rust was merged in the kernel),
we added `-Wclippy::dbg_macro` to the compilation flags. This worked
great with our custom `dbg!` macro (vendored from `std`, but slightly
modified to use the kernel printing facilities).

However, in the very next version, 1.61.0, it stopped working [1] since
the lint started to use a Rust diagnostic item rather than a path to find
the `dbg!` macro [1]. This behavior remains until the current nightly
(1.83.0).

Therefore, currently, the `dbg_macro` is not doing anything, which
explains why we can invoke `dbg!` in samples/rust/rust_print.rs`, as well
as why changing the `#[allow()]`s to `#[expect()]`s in `std_vendor.rs`
doctests does not work since they are not fulfilled.

One possible workaround is using `rustc_attrs` like the standard library
does. However, this is intended to be internal, and we just started
supporting several Rust compiler versions, so it is best to avoid it.

Therefore, instead, use `disallowed_macros`. It is a stable lint and
is more flexible (in that we can provide different macros), although
its diagnostic message(s) are not as nice as the specialized one (yet),
and does not allow to set different lint levels per macro/path [2].

In turn, this requires allowing the (intentional) `dbg!` use in the
sample, as one would have expected.

Finally, in a single case, the `allow` is fixed to be an inner attribute,
since otherwise it was not being applied.

Link: rust-lang/rust-clippy#11303 [1]
Link: rust-lang/rust-clippy#11307 [2]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to ojeda/linux that referenced this issue Sep 4, 2024
@ojeda
rust: replace clippy::dbg_macro with disallowed_macros
cbdd221 
Back when we used Rust 1.60.0 (before Rust was merged in the kernel),
we added `-Wclippy::dbg_macro` to the compilation flags. This worked
great with our custom `dbg!` macro (vendored from `std`, but slightly
modified to use the kernel printing facilities).

However, in the very next version, 1.61.0, it stopped working [1] since
the lint started to use a Rust diagnostic item rather than a path to find
the `dbg!` macro [1]. This behavior remains until the current nightly
(1.83.0).

Therefore, currently, the `dbg_macro` is not doing anything, which
explains why we can invoke `dbg!` in samples/rust/rust_print.rs`, as well
as why changing the `#[allow()]`s to `#[expect()]`s in `std_vendor.rs`
doctests does not work since they are not fulfilled.

One possible workaround is using `rustc_attrs` like the standard library
does. However, this is intended to be internal, and we just started
supporting several Rust compiler versions, so it is best to avoid it.

Therefore, instead, use `disallowed_macros`. It is a stable lint and
is more flexible (in that we can provide different macros), although
its diagnostic message(s) are not as nice as the specialized one (yet),
and does not allow to set different lint levels per macro/path [2].

In turn, this requires allowing the (intentional) `dbg!` use in the
sample, as one would have expected.

Finally, in a single case, the `allow` is fixed to be an inner attribute,
since otherwise it was not being applied.

Link: rust-lang/rust-clippy#11303 [1]
Link: rust-lang/rust-clippy#11307 [2]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to ojeda/linux that referenced this issue Oct 3, 2024
@ojeda
rust: replace clippy::dbg_macro with disallowed_macros
e885f74 
Back when we used Rust 1.60.0 (before Rust was merged in the kernel),
we added `-Wclippy::dbg_macro` to the compilation flags. This worked
great with our custom `dbg!` macro (vendored from `std`, but slightly
modified to use the kernel printing facilities).

However, in the very next version, 1.61.0, it stopped working [1] since
the lint started to use a Rust diagnostic item rather than a path to find
the `dbg!` macro [1]. This behavior remains until the current nightly
(1.83.0).

Therefore, currently, the `dbg_macro` is not doing anything, which
explains why we can invoke `dbg!` in samples/rust/rust_print.rs`, as well
as why changing the `#[allow()]`s to `#[expect()]`s in `std_vendor.rs`
doctests does not work since they are not fulfilled.

One possible workaround is using `rustc_attrs` like the standard library
does. However, this is intended to be internal, and we just started
supporting several Rust compiler versions, so it is best to avoid it.

Therefore, instead, use `disallowed_macros`. It is a stable lint and
is more flexible (in that we can provide different macros), although
its diagnostic message(s) are not as nice as the specialized one (yet),
and does not allow to set different lint levels per macro/path [2].

In turn, this requires allowing the (intentional) `dbg!` use in the
sample, as one would have expected.

Finally, in a single case, the `allow` is fixed to be an inner attribute,
since otherwise it was not being applied.

Link: rust-lang/rust-clippy#11303 [1]
Link: rust-lang/rust-clippy#11307 [2]
Tested-by: Gary Guo <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to Rust-for-Linux/linux that referenced this issue Oct 7, 2024
@ojeda
rust: replace clippy::dbg_macro with disallowed_macros
7b95f6d 
Back when we used Rust 1.60.0 (before Rust was merged in the kernel),
we added `-Wclippy::dbg_macro` to the compilation flags. This worked
great with our custom `dbg!` macro (vendored from `std`, but slightly
modified to use the kernel printing facilities).

However, in the very next version, 1.61.0, it stopped working [1] since
the lint started to use a Rust diagnostic item rather than a path to find
the `dbg!` macro [1]. This behavior remains until the current nightly
(1.83.0).

Therefore, currently, the `dbg_macro` is not doing anything, which
explains why we can invoke `dbg!` in samples/rust/rust_print.rs`, as well
as why changing the `#[allow()]`s to `#[expect()]`s in `std_vendor.rs`
doctests does not work since they are not fulfilled.

One possible workaround is using `rustc_attrs` like the standard library
does. However, this is intended to be internal, and we just started
supporting several Rust compiler versions, so it is best to avoid it.

Therefore, instead, use `disallowed_macros`. It is a stable lint and
is more flexible (in that we can provide different macros), although
its diagnostic message(s) are not as nice as the specialized one (yet),
and does not allow to set different lint levels per macro/path [2].

In turn, this requires allowing the (intentional) `dbg!` use in the
sample, as one would have expected.

Finally, in a single case, the `allow` is fixed to be an inner attribute,
since otherwise it was not being applied.

Link: rust-lang/rust-clippy#11303 [1]
Link: rust-lang/rust-clippy#11307 [2]
Tested-by: Gary Guo <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to Rust-for-Linux/linux that referenced this issue Oct 7, 2024
@ojeda
rust: replace clippy::dbg_macro with disallowed_macros
8577c9d 
Back when we used Rust 1.60.0 (before Rust was merged in the kernel),
we added `-Wclippy::dbg_macro` to the compilation flags. This worked
great with our custom `dbg!` macro (vendored from `std`, but slightly
modified to use the kernel printing facilities).

However, in the very next version, 1.61.0, it stopped working [1] since
the lint started to use a Rust diagnostic item rather than a path to find
the `dbg!` macro [1]. This behavior remains until the current nightly
(1.83.0).

Therefore, currently, the `dbg_macro` is not doing anything, which
explains why we can invoke `dbg!` in samples/rust/rust_print.rs`, as well
as why changing the `#[allow()]`s to `#[expect()]`s in `std_vendor.rs`
doctests does not work since they are not fulfilled.

One possible workaround is using `rustc_attrs` like the standard library
does. However, this is intended to be internal, and we just started
supporting several Rust compiler versions, so it is best to avoid it.

Therefore, instead, use `disallowed_macros`. It is a stable lint and
is more flexible (in that we can provide different macros), although
its diagnostic message(s) are not as nice as the specialized one (yet),
and does not allow to set different lint levels per macro/path [2].

In turn, this requires allowing the (intentional) `dbg!` use in the
sample, as one would have expected.

Finally, in a single case, the `allow` is fixed to be an inner attribute,
since otherwise it was not being applied.

Link: rust-lang/rust-clippy#11303 [1]
Link: rust-lang/rust-clippy#11307 [2]
Tested-by: Gary Guo <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ojeda @Centri3