Broken build after updating: `coverage` is ambiguous; ambiguous because of a name conflict with a builtin attribute
NicholasGorski opened this issue · 10 comments
Code
I have an internal macro for domain-specific coverage tracking:
macro_rules! coverage {
() => {
/* .. */
};
}
pub(crate) use coverage;On version 1.73.0, this compiles without error (godbolt).
On version 1.74.0 and above, this results in an error (godbolt):
error[E0659]: `coverage` is ambiguous
--> <source>:7:16
|
7 | pub(crate) use coverage;
| ^^^^^^^^ ambiguous name
|
= note: ambiguous because of a name conflict with a builtin attribute
= note: `coverage` could refer to a built-in attribute
My understanding is that this is an acceptable breakage in general. (Though, perhaps should be in the 1.74.0 compatibility notes).
However, what might make this unacceptable is that this attribute is not actually stable:
#[coverage(off)]
pub fn foo() {}gives (godbolt):
error[E0658]: the `#[coverage]` attribute is an experimental feature
--> <source>:1:1
|
1 | #[coverage(off)]
| ^^^^^^^^^^^^^^^^
|
= note: see issue #84605 <https://github.com/rust-lang/rust/issues/84605> for more information
= help: add `#![feature(coverage_attribute)]` to the crate
Is it expected that an inactive experimental attribute can cause name ambiguity on stable Rust?
searched nightlies: from nightly-2023-08-20 to nightly-2024-02-17
regressed nightly: nightly-2023-09-15
searched commit range: 8142a31...ca2b74f
regressed commit: c728bf3
bisected with cargo-bisect-rustc v0.6.7
Host triple: x86_64-unknown-linux-gnu
Reproduce with:
cargo bisect-rustc --start=1.73.0 --without-cargo --preserve --script=./test.sh cc @bossmc @oli-obk was this a slip or was intentional and just missed the release notes?
WG-prioritization assigning priority (Zulip discussion).
@rustbot label -I-prioritize +P-medium
Definitely unintentional. But it also means the feature gating of no_coverage (and possibly other builtin unstable attributes), aren't correctly gated
Totally agree with oli-obk, I wouldn't expect/want this to impact stable rust until the feature stabilizes. I'm not an expert on the feature-gating system in rustc, but I'd hope there's a way to ignore attributes "hidden" behind unstable features.
In general, is it ok to introduce new attributes like this? This causes compile errors by reserving a name in the "global" macro namespace it feels like it's quite similar to defining a new keyword (not quite as limiting maybe, since it only clashes in the macro namespace, but there's no way to not import builtin attributes). Is this a thing that's been considered before? Does stabilization need to wait for edition 2024?
Interestingly https://doc.rust-lang.org/reference/names/namespaces.html#sub-namespaces suggests these two macros are in different namespaces (attributes and bang-macros are separate sub-namespaces) but that use is still not allowed to cause a clash (which makes me wonder how, outside the builtin macro/attributes, you can end up with an attribute and a macro in scope at the same time)
Yea, we could fix this for the unstable feature gating, but once it is stabilized it is still going to clash.
(This is a surprising semver hazard I never really considered.)
Unfortunately I don't remember if there is a reason for that sub-namespace name collision. I would expect intentional use of use for a built-in attribute would be extremely rare.
Intentional use of a built-in attribute likely is rare, but defining a macro with the same name as a built-in attribute (then trying to use it) might be a little more common.
Weirdly, this doesn't reproduce with the #[test] attribute - https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=30a2246475f218078c6be85f2cde3ced so there's something special going on with the coverage attribute's registration?
test is similar to a proc-macro, which is exposed via the prelude. It is not a "built-in" attribute.
The reference hasn't really been updated from when that changed. The sub-namespace section also probably should be clearer on what it means to shadow. I also don't have a good explanation why a prelude attribute is treated differently from a built-in one.
Right you are, changing test to allow causes the "ambiguous because of a name conflict with a builtin attribute" error.