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

disallowed_macros false negatives #11431

Open
ojeda opened this issue Aug 30, 2023 · 1 comment
Open

disallowed_macros false negatives #11431

ojeda opened this issue Aug 30, 2023 · 1 comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't

Comments

@ojeda
Copy link
Contributor

ojeda commented Aug 30, 2023

Summary

disallowed_macros has several false negatives. See reproducer.

Lint Name

disallowed_macros

Reproducer

With clippy.toml:

disallowed-macros = [ "x::m" ]

And a command line like:

clippy-driver --edition=2021 -Dclippy::disallowed_macros x.rs

This works (similar to an existing test in Clippy):

macro_rules! m (
    ($t:tt) => (1)
);

fn main() {
    let _ = m!(42);
}

However, this does not:

macro_rules! m (
    ($t:tt) => ($t)
);

fn main() {
    let _ = m!(42);
}

Nor:

macro_rules! m (
    ($t:tt) => ($t + $t)
);

fn main() {
    let _ = m!(42);
}

In addition, empty expansions also do not work -- this may be due to the lint not having a pre-expansion part, but still, it would be nice to handle:

macro_rules! m (
    ($t:tt) => ()
);

fn main() {
    m!(42);
}

Version

rustc 1.72.0 (5680fa18f 2023-08-23)
rustc 1.74.0-nightly (84a9f4c6e 2023-08-29)
@ojeda ojeda added C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't labels Aug 30, 2023
@Alexendoo
Copy link
Member

Opened #11439 for ($t:tt) => ($t + $t)

($t:tt) => ($t) doesn't get detected because it doesn't produce any of its own tokens, from the HIR's perspective m!(42) is the same as 42. A pre expansion pass would be able to see an invocation of it as long it's not part of another macro expansion, but it's a late lint because we need to resolve the macro paths to definitions

($t:tt) => () does produce an empty item in the AST, but those are not preserved in the HIR so we don't pick it up

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Dec 14, 2023
This is the next upgrade to the Rust toolchain, from 1.73.0 to 1.74.1
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

# Unstable features

No unstable features (that we use) were stabilized.

Therefore, the only unstable features allowed to be used outside the
`kernel` crate are still `new_uninit,offset_of`, though other code to
be upstreamed may increase the list (e.g. `offset_of` was added recently).

Please see [3] for details.

# Other improvements

Rust 1.74.0 allows to use `#[repr(Rust)]` explicitly [4], which can be
useful to be explicit about particular cases that would normally use
e.g. the C representation, such as silencing lints like the upcoming
additions we requested [5] to the `no_mangle_with_rust_abi` Clippy lint
(which in turn triggered the `#[repr(Rust)]` addition).

Rust 1.74.0 includes a fix for one of the false negative cases we reported
in Clippy's `disallowed_macros` lint [6] that we would like to use in
the future.

Rust 1.74.1 fixes an ICE that the Apple AGX GPU driver was hitting [7].

# Required changes

For this upgrade, no changes were required (i.e. on our side).

# `alloc` upgrade and reviewing

The vast majority of changes are due to our `alloc` fork being upgraded
at once.

There are two kinds of changes to be aware of: the ones coming from
upstream, which we should follow as closely as possible, and the updates
needed in our added fallible APIs to keep them matching the newer
infallible APIs coming from upstream.

Instead of taking a look at the diff of this patch, an alternative
approach is reviewing a diff of the changes between upstream `alloc` and
the kernel's. This allows to easily inspect the kernel additions only,
especially to check if the fallible methods we already have still match
the infallible ones in the new version coming from upstream.

Another approach is reviewing the changes introduced in the additions in
the kernel fork between the two versions. This is useful to spot
potentially unintended changes to our additions.

To apply these approaches, one may follow steps similar to the following
to generate a pair of patches that show the differences between upstream
Rust and the kernel (for the subset of `alloc` we use) before and after
applying this patch:

    # Get the difference with respect to the old version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > old.patch
    git -C linux restore rust/alloc

    # Apply this patch.
    git -C linux am rust-upgrade.patch

    # Get the difference with respect to the new version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > new.patch
    git -C linux restore rust/alloc

Now one may check the `new.patch` to take a look at the additions (first
approach) or at the difference between those two patches (second
approach). For the latter, a side-by-side tool is recommended.

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1741-2023-12-07 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: Rust-for-Linux#2 [3]
Link: rust-lang/rust#114201 [4]
Link: rust-lang/rust-clippy#11219 [5]
Link: rust-lang/rust-clippy#11431 [6]
Link: rust-lang/rust#117976 (comment) [7]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
fbq pushed a commit to Rust-for-Linux/linux that referenced this issue Dec 14, 2023
This is the next upgrade to the Rust toolchain, from 1.73.0 to 1.74.1
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

# Unstable features

No unstable features (that we use) were stabilized.

Therefore, the only unstable features allowed to be used outside the
`kernel` crate are still `new_uninit,offset_of`, though other code to
be upstreamed may increase the list (e.g. `offset_of` was added recently).

Please see [3] for details.

# Other improvements

Rust 1.74.0 allows to use `#[repr(Rust)]` explicitly [4], which can be
useful to be explicit about particular cases that would normally use
e.g. the C representation, such as silencing lints like the upcoming
additions we requested [5] to the `no_mangle_with_rust_abi` Clippy lint
(which in turn triggered the `#[repr(Rust)]` addition).

Rust 1.74.0 includes a fix for one of the false negative cases we reported
in Clippy's `disallowed_macros` lint [6] that we would like to use in
the future.

Rust 1.74.1 fixes an ICE that the Apple AGX GPU driver was hitting [7].

# Required changes

For this upgrade, no changes were required (i.e. on our side).

# `alloc` upgrade and reviewing

The vast majority of changes are due to our `alloc` fork being upgraded
at once.

There are two kinds of changes to be aware of: the ones coming from
upstream, which we should follow as closely as possible, and the updates
needed in our added fallible APIs to keep them matching the newer
infallible APIs coming from upstream.

Instead of taking a look at the diff of this patch, an alternative
approach is reviewing a diff of the changes between upstream `alloc` and
the kernel's. This allows to easily inspect the kernel additions only,
especially to check if the fallible methods we already have still match
the infallible ones in the new version coming from upstream.

Another approach is reviewing the changes introduced in the additions in
the kernel fork between the two versions. This is useful to spot
potentially unintended changes to our additions.

To apply these approaches, one may follow steps similar to the following
to generate a pair of patches that show the differences between upstream
Rust and the kernel (for the subset of `alloc` we use) before and after
applying this patch:

    # Get the difference with respect to the old version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > old.patch
    git -C linux restore rust/alloc

    # Apply this patch.
    git -C linux am rust-upgrade.patch

    # Get the difference with respect to the new version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > new.patch
    git -C linux restore rust/alloc

Now one may check the `new.patch` to take a look at the additions (first
approach) or at the difference between those two patches (second
approach). For the latter, a side-by-side tool is recommended.

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1741-2023-12-07 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: #2 [3]
Link: rust-lang/rust#114201 [4]
Link: rust-lang/rust-clippy#11219 [5]
Link: rust-lang/rust-clippy#11431 [6]
Link: rust-lang/rust#117976 (comment) [7]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Link: https://lore.kernel.org/r/20231214092958.377061-1-ojeda@kernel.org
UtsavBalar1231 pushed a commit to UtsavBalar1231/kernel_rockchip_linux that referenced this issue Dec 18, 2023
This is the next upgrade to the Rust toolchain, from 1.73.0 to 1.74.1
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

# Unstable features

No unstable features (that we use) were stabilized.

Therefore, the only unstable features allowed to be used outside the
`kernel` crate are still `new_uninit,offset_of`, though other code to
be upstreamed may increase the list (e.g. `offset_of` was added recently).

Please see [3] for details.

# Other improvements

Rust 1.74.0 allows to use `#[repr(Rust)]` explicitly [4], which can be
useful to be explicit about particular cases that would normally use
e.g. the C representation, such as silencing lints like the upcoming
additions we requested [5] to the `no_mangle_with_rust_abi` Clippy lint
(which in turn triggered the `#[repr(Rust)]` addition).

Rust 1.74.0 includes a fix for one of the false negative cases we reported
in Clippy's `disallowed_macros` lint [6] that we would like to use in
the future.

Rust 1.74.1 fixes an ICE that the Apple AGX GPU driver was hitting [7].

# Required changes

For this upgrade, no changes were required (i.e. on our side).

# `alloc` upgrade and reviewing

The vast majority of changes are due to our `alloc` fork being upgraded
at once.

There are two kinds of changes to be aware of: the ones coming from
upstream, which we should follow as closely as possible, and the updates
needed in our added fallible APIs to keep them matching the newer
infallible APIs coming from upstream.

Instead of taking a look at the diff of this patch, an alternative
approach is reviewing a diff of the changes between upstream `alloc` and
the kernel's. This allows to easily inspect the kernel additions only,
especially to check if the fallible methods we already have still match
the infallible ones in the new version coming from upstream.

Another approach is reviewing the changes introduced in the additions in
the kernel fork between the two versions. This is useful to spot
potentially unintended changes to our additions.

To apply these approaches, one may follow steps similar to the following
to generate a pair of patches that show the differences between upstream
Rust and the kernel (for the subset of `alloc` we use) before and after
applying this patch:

    # Get the difference with respect to the old version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > old.patch
    git -C linux restore rust/alloc

    # Apply this patch.
    git -C linux am rust-upgrade.patch

    # Get the difference with respect to the new version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > new.patch
    git -C linux restore rust/alloc

Now one may check the `new.patch` to take a look at the additions (first
approach) or at the difference between those two patches (second
approach). For the latter, a side-by-side tool is recommended.

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1741-2023-12-07 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: Rust-for-Linux/linux#2 [3]
Link: rust-lang/rust#114201 [4]
Link: rust-lang/rust-clippy#11219 [5]
Link: rust-lang/rust-clippy#11431 [6]
Link: rust-lang/rust#117976 (comment) [7]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Link: https://lore.kernel.org/r/20231214092958.377061-1-ojeda@kernel.org
ojeda added a commit to ojeda/linux that referenced this issue Dec 21, 2023
This is the next upgrade to the Rust toolchain, from 1.73.0 to 1.74.1
(i.e. the latest) [1].

See the upgrade policy [2] and the comments on the first upgrade in
commit 3ed03f4 ("rust: upgrade to Rust 1.68.2").

# Unstable features

No unstable features (that we use) were stabilized.

Therefore, the only unstable features allowed to be used outside the
`kernel` crate are still `new_uninit,offset_of`, though other code to
be upstreamed may increase the list (e.g. `offset_of` was added recently).

Please see [3] for details.

# Other improvements

Rust 1.74.0 allows to use `#[repr(Rust)]` explicitly [4], which can be
useful to be explicit about particular cases that would normally use
e.g. the C representation, such as silencing lints like the upcoming
additions we requested [5] to the `no_mangle_with_rust_abi` Clippy lint
(which in turn triggered the `#[repr(Rust)]` addition).

Rust 1.74.0 includes a fix for one of the false negative cases we reported
in Clippy's `disallowed_macros` lint [6] that we would like to use in
the future.

Rust 1.74.1 fixes an ICE that the Apple AGX GPU driver was hitting [7].

# Required changes

For this upgrade, no changes were required (i.e. on our side).

# `alloc` upgrade and reviewing

The vast majority of changes are due to our `alloc` fork being upgraded
at once.

There are two kinds of changes to be aware of: the ones coming from
upstream, which we should follow as closely as possible, and the updates
needed in our added fallible APIs to keep them matching the newer
infallible APIs coming from upstream.

Instead of taking a look at the diff of this patch, an alternative
approach is reviewing a diff of the changes between upstream `alloc` and
the kernel's. This allows to easily inspect the kernel additions only,
especially to check if the fallible methods we already have still match
the infallible ones in the new version coming from upstream.

Another approach is reviewing the changes introduced in the additions in
the kernel fork between the two versions. This is useful to spot
potentially unintended changes to our additions.

To apply these approaches, one may follow steps similar to the following
to generate a pair of patches that show the differences between upstream
Rust and the kernel (for the subset of `alloc` we use) before and after
applying this patch:

    # Get the difference with respect to the old version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > old.patch
    git -C linux restore rust/alloc

    # Apply this patch.
    git -C linux am rust-upgrade.patch

    # Get the difference with respect to the new version.
    git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
    git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
        cut -d/ -f3- |
        grep -Fv README.md |
        xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
    git -C linux diff --patch-with-stat --summary -R > new.patch
    git -C linux restore rust/alloc

Now one may check the `new.patch` to take a look at the additions (first
approach) or at the difference between those two patches (second
approach). For the latter, a side-by-side tool is recommended.

Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1741-2023-12-07 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: Rust-for-Linux#2 [3]
Link: rust-lang/rust#114201 [4]
Link: rust-lang/rust-clippy#11219 [5]
Link: rust-lang/rust-clippy#11431 [6]
Link: rust-lang/rust#117976 (comment) [7]
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Tested-by: David Gow <davidgow@google.com>
Link: https://lore.kernel.org/r/20231214092958.377061-1-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't
Projects
None yet
Development

No branches or pull requests

2 participants