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

Inconsistent warning for macros that contain unexpected_cfgs #132572

Closed
funemy opened this issue Nov 3, 2024 · 7 comments · Fixed by #132577
Closed

Inconsistent warning for macros that contain unexpected_cfgs #132572

funemy opened this issue Nov 3, 2024 · 7 comments · Fixed by #132577
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-unexpected_cfgs Lint: unexpected_cfgs T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@funemy
Copy link

funemy commented Nov 3, 2024

Problem

Hi, I recently encountered an inconsistent behavior of cargo check when I have a macro which expands to something with unexpected_cfgs.

In short, I have a macro definition that expand to something with #[cfg(feature = "my_feature")]. When my_feature is not declared in Cargo.toml of my crate, I expected unexpected_cfgs to be reported whenever I use this macro.
However, whenever I use this macro in a downstream project (which also doesn't declare my_feature), no warning reported.

I originally thought this was a rust-analyzer problem, but after the discussion in rust-lang/rust-analyzer#18461, it seems this is more relevant to cargo.

I would like to understand if this is the expected behavior, because it feels like a bug to me.

Steps

  1. Say I have a library mylib, and mylib/lib.rs has
pub fn my_lib_func() {
    println!("my nice little library")
}

#[macro_export]
macro_rules! my_lib_macro {
    () => {
        #[cfg(feature = "my_feature")]
        $crate::my_lib_func()
    };
}

#[test]
fn my_unit_test() {
    // rust-analyzer warning here
    my_lib_macro!();
}
  1. Now running cargo check --tests, I got the expected warning:
  --> mylib/src/lib.rs:8:15
   |
8  |         #[cfg(feature = "my_feature")]
   |               ^^^^^^^^^^^^^^^^^^^^^^ help: remove the condition
...
15 |     my_lib_macro!();
   |     --------------- in this macro invocation
   |
   = note: no expected values for `feature`
   = help: consider adding `my_feature` as a feature in `Cargo.toml`
   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
   = note: `#[warn(unexpected_cfgs)]` on by default
   = note: this warning originates in the macro `my_lib_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
  1. However, if I use mylib in other packages, say I have another crate, myapp, that depends on mylib.
    In myapp/main.rs, I have:
use mylib::my_lib_macro;

fn main() {
    my_lib_macro!();
}
  1. Running cargo check on myapp reports nothing.

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.82.0 (8f40fc59f 2024-08-21)
release: 1.82.0
commit-hash: 8f40fc59fb0c8df91c97405785197f3c630304ea
commit-date: 2024-08-21
host: x86_64-apple-darwin
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.6.0 (sys:0.4.74+curl-8.9.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w 11 Sep 2023
os: Mac OS 14.5.0 [64-bit]

@funemy funemy added the C-bug Category: This is a bug. label Nov 3, 2024
@funemy funemy changed the title Inconsistent warning for macros that contain unexpected_cfgs Inconsistent warning for macros that contain unexpected_cfgs Nov 3, 2024
@weihanglo
Copy link
Member

weihanglo commented Nov 3, 2024

It doesn't need to have multiple Cargo packages. Simply a src/main.rs and a src/lib.rs in a single package can reproduce it.

This also applies to some simpler lints like unused_variables. It won't report the lint diagnostic from the caller crate. Change the macro to let x = 1 and you'll see.

However, if you change the macro content to 1_i32 << 32; you'll get a lint error of when running cargo c --bins

warning: unused bitwise operation that must be used
 --> src/main.rs:3:5
  |
3 |     foo::my_macro!();
  |     ^^^^^^^^^^^^^^^^ the bitwise operation produces a value
  |
  = note: `#[warn(unused_must_use)]` on by default
  = note: this warning originates in the macro `foo::my_macro` (in Nightly builds, run with -Z macro-backtrace for more info)

Despite that, you won't get the arithmetic_overflow lint error which you could get from running cargo t --lib --no-run.

error: this arithmetic operation will overflow
  --> src/lib.rs:4:9
   |
4  |         1_i32 << 32;
   |         ^^^^^^^^^^^ attempt to shift left by `32_i32`, which would overflow
...
11 |     my_macro!();
   |     ----------- in this macro invocation
   |
   = note: `#[deny(arithmetic_overflow)]` on by default
   = note: this error originates in the macro `my_macro` (in Nightly builds, run with -Z macro-backtrace for more info)

I feel like this behavior pretty much depends on how the lint is implemented, though not a lint wizard so may need experts to answer this.

cc @Urgau do you happen to know how unexpected_cfgs is expected to work for macros from external crates?

@weihanglo weihanglo added the A-diagnostics Area: Messages for errors, warnings, and lints label Nov 3, 2024
@Urgau
Copy link
Member

Urgau commented Nov 3, 2024

I'm pretty sure this is because we (the compiler) don't (in general) show warnings coming from external macro, and the unexpected_cfgs is not marked as being allowed to report on code generated from external macros.

This is done to avoid awkward warnings where the users can't do anything about the warnings.

In the case of the unexpected_cfgs lint I could see it being useful to marked as reportable in external macros.

@Urgau
Copy link
Member

Urgau commented Nov 3, 2024

I'm transferring this issue to main Rust repo, as it's an issue with compiler.

@rustbot transfer rust EDIT: nevermind I can't, @weihanglo would you mind doing it for me

@rustbot

This comment has been minimized.

1 similar comment
@rustbot

This comment has been minimized.

@weihanglo

This comment has been minimized.

@rustbot

This comment has been minimized.

@weihanglo weihanglo transferred this issue from rust-lang/cargo Nov 3, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. L-unexpected_cfgs Lint: unexpected_cfgs A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Nov 4, 2024
bors added a commit to rust-lang/cargo that referenced this issue Nov 4, 2024
Enable transfer feature in triagebot

This PR enables the `transfer` feature from triagebot, it permits issue transfers within the org.

Documentation at: https://forge.rust-lang.org/triagebot/transfer.html

It would have been useful in rust-lang/rust#132572 (comment)

r? `@ehuss`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 9, 2024
…o, r=petrochenkov

Report the `unexpected_cfgs` lint in external macros

This PR marks the `unexpected_cfgs` lint as being reportable in external macros, as it's probably not the intention of the macro author to leave ineffective cfgs in the users code.

Fixes rust-lang#132572
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 10, 2024
… r=<try>

Report the `unexpected_cfgs` lint in external macros

This PR marks the `unexpected_cfgs` lint as being reportable in external macros, as it's probably not the intention of the macro author to leave ineffective cfgs in the users code.

Fixes rust-lang#132572

try-job: aarch64-gnu-debug
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 16, 2024
… r=<try>

Report the `unexpected_cfgs` lint in external macros

This PR marks the `unexpected_cfgs` lint as being reportable in external macros, as it's probably not the intention of the macro author to leave ineffective cfgs in the users code.

Fixes rust-lang#132572

try-job: aarch64-gnu-debug
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 18, 2024
…o, r=petrochenkov

Report the `unexpected_cfgs` lint in external macros

This PR marks the `unexpected_cfgs` lint as being reportable in external macros, as it's probably not the intention of the macro author to leave ineffective cfgs in the users code.

Fixes rust-lang#132572

try-job: aarch64-gnu-debug
@bors bors closed this as completed in 4720054 Nov 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 19, 2024
Rollup merge of rust-lang#132577 - Urgau:check-cfg-report-extern-macro, r=petrochenkov

Report the `unexpected_cfgs` lint in external macros

This PR marks the `unexpected_cfgs` lint as being reportable in external macros, as it's probably not the intention of the macro author to leave ineffective cfgs in the users code.

Fixes rust-lang#132572

try-job: aarch64-gnu-debug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-unexpected_cfgs Lint: unexpected_cfgs T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants