Remove `cargo::rustc-check-cfg` to only have one place to configure the `unknown_cfgs` lint

[tbu-]

There are currently two places where the lint can be configured, in build.rs using cargo::rustc-check-cfg and in the Cargo.toml in the lints section. Generally, having only one place is better than having two, as users don't have to look into two places to find the configuration.

Since this feature hasn't been shipped to a stable compiler yet, we could currently still remove one of them without backward compatibility hazards.

The possible cfgs of a crate across all build configurations are known statically, since they're at most all that appear in the source code. This means that all possible configurations should be expressible in Cargo.toml.

[tbu-]

The declarative approach also works for cfgs used in build.rs directly: https://github.com/rust-lang/rust/issues/125368.

[epage]

Currently, we have it documented to encourage

• build.rs if you specify the associated cfg in build.rs
• Cargo.toml for other cases (like fuzzing)

When we document the build.rs case, we talk about the importance of keeping cargo::rustc-check-cfg close to cargo::rustc-cfg. I think this is an important property to help ensure they are updated hand-in-hand.

[mcclure]

@epage , please do notice my note at the end in #125368 — the fact that the build-rs-printf method does not suppress warnings creates a problem because currently it's possible for the warning to recommend a mitigation which has no effect.

Assuming that isn't fixable, if you do keep in the build-rs-printf method, I think you should be careful to document build-rs-printf will not helpful if cfgs are used in build.rs itself, and ideally you should modify the warning hint to not recommend the build-rs-printf fix if it detects (because this is detectable) that the warning has been triggered within build.rs. Alternately, removing the build-rs-printf method would mean you don't need that complexity in messaging/warning plumbing.

[Urgau]

I agree with @epage, keeping cargo::rustc-check-cfg is important, there are users how would be negatively affected by it's removal, like users of the cfg_aliases, which would need to manually declare the expected cfgs outside of where they define their aliases, which goes completely against what we are trying to achieve.

Assuming that isn't fixable

It is fixable. I just didn't yet have time to do it.

[Urgau]

The declarative approach also works for cfgs used in build.rs directly: rust-lang/rust#125368.

While it works, it's also not 100% the same thing, the [lints.rust.unexpected_cfgs.check-cfg] lint config applies to very unit, including build.rs, but if the config is declared by the build.rs, that probably means it isn't expected for the build.rs it-self. So putting it at the same place as the cargo::rustc-cfg instruction guarantees the same applicability.

[tbu-]

When we document the build.rs case, we talk about the importance of keeping cargo::rustc-check-cfg close to cargo::rustc-cfg. I think this is an important property to help ensure they are updated hand-in-hand.

I see, I think this argument makes sense. It also means that tools outputting cargo::rustc-cfg can also directly output cargo::rustc-check-cfg.

I still feel that a) having a non-declarative version of the config makes it harder for tools, because they need to run untrusted code to get a list of cfgs, and b) users need to be taught about two things that do roughly the same thing, are two significant downsides to this approach.

If we started with [lints.rust.unexpected_cfgs.check-cfg] today, would the build.rs way we be added? Perhaps yes.

[epage]

@mcclure

@epage , please do notice my note at the end in https://github.com/rust-lang/rust/issues/125368 — the fact that the build-rs-printf method does not suppress warnings creates a problem because currently it's possible for the warning to recommend a mitigation which has no effect.

build.rs only sets compilation flags for all other build targets and not for itself.

With the split I gave of static vs dynamic cfgs, could you give an example of a cfg that the build.rs both reads and sets where this is a problem?

[mcclure]

build.rs only sets compilation flags for all other build targets and not for itself.

With the split I gave of static vs dynamic cfgs, could you give an example of a cfg that the build.rs both reads and sets where this is a problem?

Hi, I'm not sure I understand what you mean by "the split I gave of static vs dynamic cfgs", but the case I give in #125368 is the case of a .rs file which for whatever reason is used both in the build.rs and also in a non-build.rs target. It's true the custom cfg can be set for only non-build.rs targets, but the rustc-check-cfg warning appears also for cfg(not()) statements, and code that runs during build.rs might indeed want to happen conditionally on cfg(not(symbolname)) where symbolname is a cfg build.rs sets.

[epage]

Thanks for the clarification.

At this point, the feature is stable. We could deprecate one but I'm not seeing enough of a case to justify it. We give a dynamic and a static way of defining these, allowing people to use the one that works well for what they are doing.

As such I propose we close this.

[weihanglo]

Second and close. Thanks everyone.