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

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

Closed
mcclure opened this issue May 21, 2024 · 5 comments · Fixed by #125412
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-check-cfg --check-cfg

Comments

@mcclure
Copy link

mcclure commented May 21, 2024

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

  • I have not experimented with disabling the warning with #[warn(unexpected_cfgs)]. The warning seems good because it catches typos. I just don't want to be warned about exe because it's not a typo.
  • In the repro I use the single colon version of rustc-check-cfg, because that feature only works when Rust MSRV is above 1.77, and requiring Rust 1.80 in order to banish a warning that didn't exist before Rust 1.80 seems silly. However, I did test with double colon and it didn't behave any different.
  • 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.
  • In case you were wondering, yes I see the warning both on exe and not(exe).

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
@mcclure mcclure added the C-bug Category: This is a bug. label May 21, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 21, 2024
@Noratrieb
Copy link
Member

@Urgau

@wesleywiser wesleywiser added the F-check-cfg --check-cfg label May 21, 2024
@wesleywiser
Copy link
Member

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
Copy link
Member

Urgau commented May 21, 2024

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.

@mcclure
Copy link
Author

mcclure commented May 21, 2024

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

@mcclure mcclure changed the title New nightly feature "rustc-check-cfg" doesn't work as advertised (cannot dismiss warning) New nightly feature "rustc-check-cfg" doesn't work as advertised by warning hint (cannot dismiss warning) May 22, 2024
@saethlin saethlin added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 22, 2024
@bors bors closed this as completed in 56427d3 May 24, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 24, 2024
Rollup merge of rust-lang#125412 - Urgau:check-cfg-less-build-rs, r=wesleywiser

Don't suggest adding the unexpected cfgs to the build-script it-self

This PR adds a check to avoid suggesting to add the unexpected cfgs inside the build-script when building the build-script it-self, as it won't have any effect, since build-scripts applies to their descended target.

Fixes rust-lang#125368
@jymchng

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. F-check-cfg --check-cfg
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants