rust-lang / rust

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

Nightly-only cfg gating like `#[cfg(target_has_atomic)]` is confusing #133295

Open jieyouxu opened 1 day ago

jieyouxu commented 1 day ago

EDIT: this is intended that cfg(target_has_atomic) is nightly-only (not feature-gated), but I find it still very confusing.


Note that #[cfg(target_has_atomic)] is not to be confused with #[cfg(target_has_atomic = "8")].

Given a snippet (just rustc --crate-type="lib", no additional --cfg flags)

#[cfg(target_has_atomic)]
fn foo() {}
warning: function `foo` is never used
 --> src/lib.rs:2:4
  |
2 | fn foo() {}
  |    ^^^
  |
  = note: `#[warn(dead_code)]` on by default

AFAICT there is no test coverage for the intended behavior of just #[cfg(target_has_atomic)] alone, other test coverage is mostly for check-cfg.

cc @Urgau since you worked on builtin-cfg rejection do you know anything about if this is intended, and if so, what is the intended behavior here?

jieyouxu commented 1 day ago

@Nemo157 has another example:

For a given snippet and rustc --cfg='foo="bar"' --crate-type=lib,

#[cfg(foo)] pub fn foo() -> usize { 1 }
#[cfg(foo = "bar")] pub fn bar() -> usize { 2 }
#[cfg(target_has_atomic)] pub fn target_has_atomic() -> usize { 3 }
#[cfg(target_has_atomic = "8")] pub fn target_has_atomic_8() -> usize { 4 }

On stable 1.82.0 this codegens

example::bar:
        mov     eax, 2
        ret
example::target_has_atomic_8:
        mov     eax, 4
        ret

On nightly 2024-11-20 this codegens

example::bar:
        mov     eax, 2
        ret
example::target_has_atomic:
        mov     eax, 3
        ret
example::target_has_atomic_8:
        mov     eax, 4
        ret

Further indicating that target_has_atomic has become active by default, but is this intentional?

taiki-e commented 1 day ago

This cfg has been added in https://github.com/rust-lang/rust/pull/106856, and means any(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", ...).

jieyouxu commented 1 day ago

Oh... This is nightly-gated but it is not feature-gated

jieyouxu commented 1 day ago

cc @joshtriplett since you r+'d #106856 do you know if there a particular reason why cfg(target_has_atomic) was nightly-gated and not feature-gated? Just asking in case there's a particular reason.

bjorn3 commented 1 day ago

All unstable cfg's are nigtly gated and not feature gated.

jieyouxu commented 1 day ago

Ah never mind, if that's intended behavior.

Urgau commented 22 hours ago

All unstable cfg's are nigtly gated and not feature gated

That's not true, most unstable cfgs are both nightly gated and feature gated, https://github.com/rust-lang/rust/blob/717f5df2c308dfb4b7b8e6c002c11fe8269c4011/compiler/rustc_feature/src/builtin_attrs.rs#L17-L40

Btw, the current cfg feature gating system assumes that all the values are gated but that doesn't map well for target_has_atomic where only the value-less should be feature gated. We would need to do extend the system to permit feature gating only some values for a cfg name.

jieyouxu commented 22 hours ago

I'm going to reopen this issue as a possible enhancement to make these kinds of gating less confusing...