Custom cfg `loom` makes cargo spam warnings

[simonwuelker]

The newest cargo nightly release includes automatic checking of expected cfg values. ¹
The TLDR is that this is supposed to catch typos like #[cfg(windosw)].

Unfortunately, this lint triggers on oneshot's loom cfg quite a lot (24 times at the time of writing).

warning: unexpected `cfg` condition name: `loom`
   --> src/lib.rs:125:11
    |
125 | #[cfg(not(loom))]
    |           ^^^^
    |
    = help: consider using a Cargo feature instead or adding `println!("cargo::rustc-check-cfg=cfg(loom)");` 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

Solutions

There are a few ways of fixing this:

• Make rustc aware of the cfg. ²
  Can be done by adding build.rs that looks like this

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

  I am personally against this, since it would be adding a whole compilation step for basically nothing.

• Add #![allow(unexpected_cfgs)] to lib.rs
  Seems lazy?

• Use a regular cargo feature instead.
  Not great because cargo features are supposed to be additive ³. But seems possible.

• Use #[cfg(test)] or some other standard-cfg instead of #[cfg(loom)]
  Haven't looked at the actual tests enough to say whether this is reasonable at all

I would be happy to implement a fix for this, any thoughts on what would be a reasonable move?

Footnotes

1. https://blog.rust-lang.org/2024/05/06/check-cfg.html ↩

2. https://doc.rust-lang.org/nightly/rustc/check-cfg.html ↩

3. https://doc.rust-lang.org/cargo/reference/features.html#feature-unification ↩

[faern]

Hmm.. None of the options seem ideal. I suggest bringing the question upstream, this pattern comes from loom itself and is nothing I invented here. Would be nice to have input from the loom developers and/or maybe have them update their recommendation and following that.

[faern]

... adding build.rs that looks like this: ..

I agree this is not nice! A build script should only be used when needed. I personally don't care as much about the compile time penalty. More the supply chain security aspect. A crate with a build.rs is a crate that can do anything at build time.

Add #![allow(unexpected_cfgs)] to lib.rs

Lazy indeed! I don't think this is an ideal solution. But it's what I'm doing temporary to get the CI working again (#36)

Use a regular cargo feature instead.

Indeed not additive. And I'm a fairly strong proponent for features being additive. Another downside is that cargo features are kind of "public features" of a crate. Loom is strictly for internal testing. This also mess with the cargo hack --feature-powerset functionality we currently use that expect loom to be either on or off for all combinations right now.

Use #[cfg(test)] or some other standard-cfg instead of #[cfg(loom)]

I don't think we are in "test mode" all the times when we want to use loom? 🤔

[simonwuelker]

Unfortunately it seems like silencing the lint is the most sane option available at the moment :/

Its what was suggested on the loom issue i filed, and other projects are doing the same:

• Bump nightly to nightly-2024-05-05 tokio-rs/tokio#6538
• Handle new unexpected_cfgs warnings microsoft/windows-rs#3022
• handle unexpected_cfgs tokio-rs/mio#1785
• chore: fix unexpected cfg warning hyperium/hyper#3660

The only real "solution" that I've seen is adding a build.rs that is not shipped on crates.io. (tokio-rs/tokio#6542) But this is bad in its own right, as you would be testing different code than what is shipped to the user...

[simonwuelker]

The blog post¹ was updated, you can now modify [lints.rust.unexpected_cfgs], like this:

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

I'll open a pr for this later

Footnotes

1. https://blog.rust-lang.org/2024/05/06/check-cfg.html#expecting-custom-cfgs ↩

[faern]

This solution seems waay cleaner! Looking forward to the PR ✨