when $expr is stringify!(...)/concat!(...), #[cfg(feature = $expr)] and #[doc = $expr] don't produce errors #53298
Comments
ExpHP
changed the title
estebank
added
A-macros
|
Hmm. This looks bad! |
|
@SergioBenitez @nrc -- any chance one of you has a clue what might have changed here? Or the desire to look into this? It seems like a pretty serious compatibility risk |
nikomatsakis
added
the
T-compiler
|
When I get home I'll try using (Well, okay, maybe the 1.17-1.18 one might not be useless...) |
|
@nikomatsakis Looking at the error and version-based diagnosis provided by @ExpHP, it seems very unlikely that this is an artifact of #34981. For one, the commit for In particular, my guess is that this was introduced with I'm not sure who, if anyone, has touched |
|
I'll start with nightly bisections. Here is the first bisect. The time range is a bit surprising (to me!): So it's in 4853584...6eb9960... but apparently it must have been reverted while This one stands out: 68c1cc6 |
|
Nightly bisection on the second change (from ICE to silently ignored) is pretty coarse. Range is 4c053db...d9f1249 with these CI commits: Almost 100% certain this change was due to pull #45464, which fixed a very familiar-looking ICE reported in #44851. |
ExpHP
changed the title
ExpHP
changed the title
ExpHP
changed the title
|
Well... I hope this was intentional, or else this compatibility risk is in fact clearly already being abused: Here is an actual test in macro_rules! a {
() => { "a" }
}
macro_rules! b {
($doc:expr) => {
#[doc = $doc]
pub struct B;
}
}
b!(a!());
fn main() {}If you run Notice that this usage of macro cc @sinkuu (In the OP I claimed that rustc ignored the |
|
Yes, |
|
That's good to hear! So I guess the title is mischaracterizing the problem once again (but I think I should stop editing it at the risk of confusing everyone's mail clients :P). |
|
Visiting for triage. It appears ambiguous as to what the intended behavior actually is here. It seems like there is a bug somewhere but I cannot immediately tell from the title/description/dialogue what the bug actually is. @petrochenkov , I am going to assign this to you for now since you seem to have the most immediate knowledge and relevant experience. The main short-term objective would be to do whatever investigation you need to do in order to make a nice summary of the core issue here (and update the title accordingly, and perhaps the bug description as well) |
|
I looked what happens, and the issue is pretty bad. This includes everything that can be succesfully parsed as a meta-item in particular. #[cfg = 10] // cfg-true
fn foo() {}
fn main() {
foo(); // OK
}The fix is simple, so I'll do it right now and submit a PR for crater run. |
Validate syntax of `cfg` attributes Fixes #53298
|
Fixed in #53893 |
Validate syntax of `cfg` attributes Fixes #53298
Reproduction
lib.rsThis should be an error, as
concat!("thing")is not allowed in this position. But instead, thecfgattribute in this example is silently ignored, andfn foogets compiled.I tested also withThe previous statement was an overgeneralization. When used on#[doc], hence why the title claims that this applies to "attributes with 'name = $expr'" in general.#[doc], this technique apparently sometimes even works!This is a regression from stable 1.17 to 1.18.
Behavior in various versions
1.0–1.17: parse error (expected behavior)1.18–1.22: ICE1.23–nightly 1.30: silently ignored! 🙈Output in 1.0–1.17 (expected)
Output in 1.18–1.22 (ICE)
Output in 1.23–nightly 1.30 (silently ignored)
The text was updated successfully, but these errors were encountered: