rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.77k stars 12.77k forks source link

New nightly feature "rustc-check-cfg" doesn't work as advertised by warning hint (cannot dismiss warning) #125368

Closed mcclure closed 6 months ago

mcclure commented 6 months ago

EDIT: Prefixing this bug with a better explanation now that I understand better: You can register a custom cfg with rustc-check-cfg in two ways, with a printf in build.rs or a [lint] in cargo.toml. However if your custom cfg is referenced in build.rs itself, the printf method does not work (you will get the warning anyway). Worse, the warning triggered by your cfg in build.rs will recommend suppressing itself by adding the printf, even though (currently?) that printf will be ineffective.


There is a very new (last 2 weeks) Nightly feature which protects against typos in cfg()s by warning if an unexpected cfg is found, comparing against a list of known Rust cfgs. That's useful, but what if your project legitimately uses a cfg that the Rust project did not forsee? According to a new compiler hint added along with this feature, you can register a custom cfg in build.rs with, for example:

    println!("cargo::rustc-check-cfg=cfg(exe)");

In my configuration, this "registration" println does not do anything.

Repro

Clone this project (branch z_bug_exe_warning), then run git submodule update --recursive --init:

https://github.com/mcclure/pocket-riscv-rs-bug/tree/z_bug_exe_warning

Build with make or cargo +nightly build --release (the former invokes the latter). You will see four instances of this warning:

warning: unexpected `cfg` condition name: `exe`
 --> src/rkyv_types.rs:4:11
  |
4 | #[cfg(not(exe))]
  |           ^^^
  |
  = help: expected names are: `clippy`, `debug_assertions`, `doc`, `docsrs`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `rustfmt`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
  = help: consider using a Cargo feature instead or adding `println!("cargo::rustc-check-cfg=cfg(exe)");` to the top of the `build.rs`
  = note: see <https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#rustc-check-cfg> for more information about checking conditional configuration
  = note: `#[warn(unexpected_cfgs)]` on by default

(Aside: In my tests, build warnings in this project are always printed three times, once with ANSI coloring and twice without, so you'll actually see 12 of this warning, but that's outside the scope of this bug and I think it's my fault).

Context: This project builds a video game for an unusual (and low-horsepower) embedded system (hence the submodule). I don't want large support libraries like PNG decoders on the device, so I preprocess large assets such as images in build.rs and serialize them using rkyv. This means I have a file, rkyv_types.rs, which contains the serialized types, and this file is included in both build.rs and the final program. So that this file can work properly in both environments, I put this in my build.rs:

    // Set this in the app build; if it's not present, we can assume we're in the build.
    println!("cargo:rustc-cfg=exe");

So this means I can use cfg(exe) to verify I'm in the built program and cfg(not(exe)) to verify I'm in build.rs. This exe is the cfg rustc-check-cfg warns about. Fair enough, let's notify rustc-check-cfg of this custom cfg so we don't get the warning. At the top of build.rs main() is this line; uncomment it:

    println!("cargo:rustc-check-cfg=cfg(exe)");

(This is of course the action recommended by the hint in the warning.) Now re-run the build with make. You will see… the exact same four errors.

The rustc-check-cfg printout does nothing.

Analysis

Meta

$ cargo +nightly --version
cargo 1.80.0-nightly (0de7f2ec6 2024-05-17)
$ uname -a
Linux Anthy 6.5.0-28-generic #29-Ubuntu SMP PREEMPT_DYNAMIC Thu Mar 28 23:46:48 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 23.10
Release:    23.10
Codename:   mantic
Noratrieb commented 6 months ago

@Urgau

wesleywiser commented 6 months ago

It seems to me that since the file is included in both build.rs and the built program, even if rustc-check-cfg suppressed the warnings about the surplus exe config in the program I might still wind up facing warnings about exe in the build.rs, since the println has not run at the time the build.rs is built. However I don't think this is the problem above, because in that case I would have expected the number of warnings to halve on adding the suppression line, not stay the same.

Your analysis here is correct, I think you just missed this tiny note which is that cargo suppresses identical warnings 🙂

Before uncommenting the cargo:rustc-cfg-check line:

...
warning: `spritetest` (build script) generated 4 warnings
warning: `spritetest` (bin "spritetest") generated 4 warnings (4 duplicates)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 11.66s

After:

...
warning: `spritetest` (build script) generated 4 warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.47s

@Urgau what's the right way to deal with check-cfg warnings originating in build.rs?

Urgau commented 6 months ago

Adding the check-cfg args directly in the lint config should do it[^1][^2].

Cargo.toml:

[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(exe)'] }

[^1]: Note that this was added very recently, around 2 nightly version ago, be sure to update to the latest nightly. [^2]: Note that using the check-cfg lint config currently produce a warning on beta, we are working on fixing that.

mcclure commented 6 months ago

Adding the check-cfg args directly in the lint config should do it12.

Cargo.toml:

[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(exe)'] }

Footnotes

  1. Note that this was added very recently, around 2 nightly version ago, be sure to update to the latest nightly.
  2. Note that using the check-cfg lint config currently produce a warning on beta, we are working on fixing that.

Thank you, I should have updated to NEWEST nightly before filing. I see that with 2024-05-20 nightly rustc the [lint] IS recommended in a hint, so this hint would have solved my problem if I had upgraded this morning before filing.

The lint works as expected and my problem is resolved (lint removes warnings where println does not).

I have a recommendation. If you are merging build.rs and executable warnings, then in principle you know that my "exe" cfg warnings are coming (in part) from build.rs. Currently, you display two recommendations*, the build.rs print and the [lint]. In the case of a cfg warning originating from a build.rs, the build.rs print! will not help and you know that ahead of time. Therefore for these warnings, I suggest you omit the build.rs print recommendation and show only the [lint] recommendation.

If that is impractical, then you may close this issue.

* with rustc 1.80.0-nightly (b92758a9a 2024-05-20) I get:

warning: unexpected `cfg` condition name: `exe`
 --> src/rkyv_types.rs:6:7
  |
6 | #[cfg(exe)]
  |       ^^^
  |
  = help: expected names are: `clippy`, `debug_assertions`, `doc`, `docsrs`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `rustfmt`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, `windows`
  = help: consider using a Cargo feature instead
  = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:
           [lints.rust]
           unexpected_cfgs = { level = "warn", check-cfg = ['cfg(exe)'] }
  = help: or consider adding `println!("cargo::rustc-check-cfg=cfg(exe)");` to the top of the `build.rs`
  = 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
jymchng commented 1 month ago

Yeah, I'm using Rust 1.81.0, within my ./.cargo/config.toml, I have:

[build]
# MUST put `rustflags` NOT `RUSTFLAGS`
rustflags=["--cfg", "MAINNET"]

Then I have a very simple ./src/main.rs:

fn main() {
    #[cfg(MAINNET)]
    println!(
        "Hello, world! from `bins` and from `MAINNET`",
    );

    #[cfg(TESTNET)]
    println!(
        "Hello, world! from `bins` and from `TESTNET`",
    );
}

These are the warnings I get from cargo check:

warning: unexpected `cfg` condition name: `MAINNET`
 --> crates/bins/simulate_txn/src/main.rs:2:11
  |
2 |     #[cfg(MAINNET)]
  |           ^^^^^^^
  |
  = help: expected names are: `clippy`, `debug_assertions`, `doc`, `docsrs`, `doctest`, `feature`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `rustfmt`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `test`, `ub_checks`, `unix`, and `windows`
  = help: consider using a Cargo feature instead
  = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:
           [lints.rust]
           unexpected_cfgs = { level = "warn", check-cfg = ['cfg(MAINNET)'] }
  = help: or consider adding `println!("cargo::rustc-check-cfg=cfg(MAINNET)");` to the top of the `build.rs`
  = 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

warning: unexpected `cfg` condition name: `TESTNET`
 --> crates/bins/simulate_txn/src/main.rs:7:11
  |
7 |     #[cfg(TESTNET)]
  |           ^^^^^^^
  |
  = help: consider using a Cargo feature instead
  = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:
           [lints.rust]
           unexpected_cfgs = { level = "warn", check-cfg = ['cfg(TESTNET)'] }
  = help: or consider adding `println!("cargo::rustc-check-cfg=cfg(TESTNET)");` to the top of the `build.rs`
  = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration

I tried adding the suggested fixes to ./build.rs:

fn main() {
    println!("cargo::rustc-check-cfg=cfg(TESTNET)");
    println!("cargo::rustc-check-cfg=cfg(MAINNET)");
}

It does not work at all.