when $expr is stringify!(...)/concat!(...), #[cfg(feature = $expr)] and #[doc = $expr] don't produce errors

[ExpHP]

Reproduction

lib.rs

macro_rules! helper {
    ($expr:expr) => {
        #[cfg(feature = $expr)]
        fn foo() {}
    }
}

helper!{concat!("thing")}

This should be an error, as concat!("thing") is not allowed in this position. But instead, the cfg attribute in this example is silently ignored, and fn foo gets compiled.

I tested also with #[doc], hence why the title claims that this applies to "attributes with 'name = $expr'" in general. The previous statement was an overgeneralization. [When used on #[doc], this technique apparently sometimes even works!](https://github.com/rust-lang/rust/issues/53298#issuecomment-415609272)

This is a regression from stable 1.17 to 1.18.

Behavior in various versions

• rustc 1.0–1.17: parse error (expected behavior)
• rustc 1.18–1.22: ICE
• rustc 1.23–nightly 1.30: silently ignored! :see_no_evil:

Output in 1.0–1.17 (expected)

src/lib.rs:3:25: 3:30 error: unexpected token: `concat!("thing")`
src/lib.rs:3         #[cfg(feature = $expr)]
                                     ^~~~~

Output in 1.18–1.22 (ICE)

$ RUST_BACKTRACE=1 cargo +1.18.0 build --example=macro-concat
   Compiling concat v0.1.0 (file:///home/lampam/cpp/throwaway/concat)
error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: run with `RUST_BACKTRACE=1` for a backtrace

thread 'rustc' panicked at 'no entry found for key', /checkout/src/libcore/option.rs:794
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
      ...
   9: core::option::expect_failed
  10: rustc_resolve::macros::<impl rustc_resolve::Resolver<'a>>::resolve_macro_to_def
  11: rustc_resolve::macros::<impl syntax::ext::base::Resolver for rustc_resolve::Resolver<'a>>::resolve_invoc
  12: syntax::ext::expand::MacroExpander::expand
  13: syntax::ext::expand::MacroExpander::expand_crate
  14: rustc_driver::driver::phase_2_configure_and_expand::{{closure}}
  15: rustc_driver::driver::phase_2_configure_and_expand
  16: rustc_driver::driver::compile_input
  17: rustc_driver::run_compiler
  18: std::panicking::try::do_call
  19: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  20: <F as alloc::boxed::FnBox<A>>::call_box
  21: std::sys::imp::thread::Thread::new::thread_start
             at /checkout/src/liballoc/boxed.rs:650
             at /checkout/src/libstd/sys_common/thread.rs:21
             at /checkout/src/libstd/sys/unix/thread.rs:84
  22: start_thread
  23: clone

Output in 1.23–nightly 1.30 (silently ignored)

   Compiling concat v0.1.0 (file:///home/lampam/cpp/throwaway/concat)
warning: function is never used: `foo`
 --> examples/macro-concat.rs:4:9
  |
4 |         fn foo() {}
  |         ^^^^^^^^
...
8 | helper!{concat!("thing")}
  | ------------------------- in this macro invocation
  |
  = note: #[warn(dead_code)] on by default

[nikomatsakis]

Hmm. This looks bad!

[nikomatsakis]

cc @SergioBenitez

[nikomatsakis]

@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

[ExpHP]

When I get home I'll try using rustc-bisect to narrow down the turning points a bit further, though it seems to me from the nature of the error's evolution that both were probably routine refactorings that took place far away from whatever part of the code should be handing this :man_shrugging:

(Well, okay, maybe the 1.17-1.18 one might not be useless...)

[SergioBenitez]

@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 allow_attr_literals landed August 2016, but versions released between then and June 8, 2017 don't appear to suffer from this issue. Of course, we'd need to test with a nightly after 8/2016 to confirm, but even the backtraces point to this being an issue elsewhere.

In particular, my guess is that this was introduced with decl_macro, the initial implementation of which landed on 5/2017, which falls perfectly inline with the diagnosis dates. What's more, the backtrace points to this being a problem involving expansion.

I'm not sure who, if anyone, has touched decl_macro since @jseyfried. @petrochenkov, perhaps you've got a greater insight?

[ExpHP]

I'll start with nightly bisections.

Here is the first bisect. The time range is a bit surprising (to me!):

nightly-2017-04-24 (2bd4b5c6d 1.18.0-nightly) ICE
nightly-2017-03-24 (e703b33e3 1.17.0-nightly) ICE    -- wait, what!?
nightly-2017-02-24 (413a975e3 1.17.0-nightly) OK
nightly-2017-03-12 (e4eb964dd 1.17.0-nightly) OK
nightly-2017-03-20 (6eb9960d3 1.17.0-nightly) ICE
nightly-2017-03-16 (0aeb9c129 1.17.0-nightly) OK
nightly-2017-03-18 (a559452b0 1.17.0-nightly) OK
nightly-2017-03-19 (485358400 1.17.0-nightly) OK

So it's in 485358400...6eb9960d3... but apparently it must have been reverted while 1.17 was in beta? (am I reading these versions right?)

This one stands out: https://github.com/rust-lang/rust/commit/68c1cc68b44bb987ec57251bc457a55292515d1d However, I see no evidence that it was reverted prior to the 1.17 release.

[ExpHP]

Nightly bisection on the second change (from ICE to silently ignored) is pretty coarse.

nightly-2017-11-20 (rustc 1.23.0-nightly (abd137ad1)) - SILENT
nightly-2017-10-20 (rustc 1.22.0-nightly (5041b3bb3)) - ICE
nightly-2017-11-01 (rustc 1.23.0-nightly (8b22e70b2)) - SILENT
nightly-2017-10-28 (rustc 1.23.0-nightly (d9f124965)) - SILENT
nightly-2017-10-24 (rustc 1.22.0-nightly (4c053db23)) - ICE
nightly-2017-10-26 - no nightly published
nightly-2017-10-25 - no nightly published
nightly-2017-10-27 - no nightly published

Range is 4c053db23...d9f124965 with these CI commits:

d9f1249655 Auto merge of #45285 - alexcrichton:update-bootstrap, r=Mark-Simulacrum
bed9a85c40 Auto merge of #45570 - nrc:manifest-no-rls, r=alexcrichton
f7b080b38e Auto merge of #45531 - steveklabnik:fix-unstable-book-formatting, r=kennytm
51456a6808 Auto merge of #45353 - wesleywiser:untracked_queries, r=michaelwoerister
1855aff8d7 Auto merge of #45524 - alexcrichton:improve-park-unpark, r=dtolnay
bb37bc1c61 Auto merge of #45523 - alexcrichton:improve-libbacktrace, r=sfackler
847f4fcf5f Auto merge of #45522 - michaelwoerister:fix-stable-hasher-cross, r=arielb1
b218a02ad8 Auto merge of #45519 - michaelwoerister:dedup-errors, r=arielb1
b0b80f8c22 Auto merge of #45380 - dotdash:arg_copies, r=arielb1
e0febe7144 Auto merge of #45488 - oli-obk:ctfe_resolve, r=eddyb
56dc171a2f Auto merge of #45464 - sinkuu:ice_44851, r=jseyfried
fa29bcedd1 Auto merge of #45096 - DSpeckhals:update-rls-data-for-save-analysis, r=alexcrichton
e847f30f57 Auto merge of #45532 - kennytm:rollup, r=kennytm
f9d2416594 Auto merge of #44636 - GuillaumeGomez:little-error-msg, r=michaelwoerister
2f5ae25a34 Auto merge of #45513 - GuillaumeGomez:rollup, r=GuillaumeGomez
f764eaf453 Auto merge of #45476 - Xanewok:fingerprint-disambiguator, r=michaelwoerister
b2478052f8 Auto merge of #45473 - SimonSapin:variance-red-green, r=nikomatsakis
6e61bbabe4 Auto merge of #45455 - kennytm:print-extern-impl-for-e0119, r=nikomatsakis
aa40292e78 Auto merge of #44603 - SimonSapin:stylo, r=alexcrichton
c2799fc9a5 Auto merge of #45446 - leodasvacas:remove-libcollections, r=alexcrichton
61af75437d Auto merge of #45350 - cjkenn:cjkenn/used-trait-imports, r=nikomatsakis
fbc3642ef1 Auto merge of #45401 - zackmdavis:crate_shorthand_visibility_modifier, r=nikomatsakis
a789fa0440 Auto merge of #44984 - Maaarcocr:master, r=nikomatsakis
336624735c Auto merge of #44766 - sunjay:lift_generics, r=nikomatsakis

Almost 100% certain this change was due to pull #45464, which fixed a very familiar-looking ICE reported in #44851.

[ExpHP]

Well... I hope this was intentional, or else this compatibility risk is in fact clearly already being abused:

---

Here is an actual test in run-pass/, added in the PR I just linked:

macro_rules! a {
    () => { "a" }
}

macro_rules! b {
    ($doc:expr) => {
        #[doc = $doc]
        pub struct B;
    }
}

b!(a!());

fn main() {}

If you run cargo doc on this, it in fact successfully documents the struct B with the text "a"!

Notice that this usage of macro b circumvents the error message that still exists in rust today when you write #[doc = a!()] directly. Something seems very not kosher about this!

cc @sinkuu

---

(In the OP I claimed that rustc ignored the #[doc] tag in this situation. Clearly the issue is more subtle than that; I updated the OP and title accordingly)

[petrochenkov]

Yes, #[doc = $doc] is actively used in the standard library. I haven't looked yet what issues it causes and why it's ignored, but #[doc = $doc] being accepted is certainly the intended behavior (at least intended by jseyfried, otherwise it kinda slipped through), plus some built-in macros apply eager expansion to their arguments (this usually happens if they expect literals), which can explain some strange cases as well.

[ExpHP]

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).

[pnkfelix]

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)

[petrochenkov]

I looked what happens, and the issue is pretty bad. Basically, cfg with any following garbage means "cfg-true" if that garbage is not a well-formed meta-item list (cfg(nested, nested, nested)), but can be... well, speculatively parsed as a meta-item without eagerly reported errors (this may happen pretty randomly, like in the example with concat!("thing") above).

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.

[petrochenkov]

Fixed in https://github.com/rust-lang/rust/pull/53893