Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICE in macro: doc meta with expr on an item, string concat, stringify!(...) #55414

Closed
awaitlink opened this issue Oct 27, 2018 · 12 comments · Fixed by #77271
Closed

ICE in macro: doc meta with expr on an item, string concat, stringify!(...) #55414

awaitlink opened this issue Oct 27, 2018 · 12 comments · Fixed by #77271
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Name resolution C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@awaitlink
Copy link
Contributor

code

macro_rules! bug {
    () => {
        bug!("bug" + stringify!(found));
    };
    ($test:expr) => {
        #[doc = $test]
        struct Test {} // looks like any item works
    };
}

bug!();

description

Any of the following actions stop the panic:

  • Removing or commenting the doc attribute:
    // #[doc = $test]
  • Replacing macro call with:
    bug!("");
  • Removing concatenation:
    () => {
        bug!(stringify!(anything));
    };
  • Replacing stringify!(...):
    () => {
        bug!("bug" + "test");
    };

stderr

Show output
thread 'main' panicked at 'no entry found for key', libcore/option.rs:1000:5
stack backtrace:
   0:     0x7f381365575e - std::sys::unix::backtrace::tracing::imp::unwind_backtrace::hf5316c67202e23c8
                               at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1:     0x7f381362e946 - std::sys_common::backtrace::print::h3068ed2a6dc7ac86
                               at libstd/sys_common/backtrace.rs:71
                               at libstd/sys_common/backtrace.rs:59
   2:     0x7f381362c54d - std::panicking::default_hook::{{closure}}::h1b9d29cc09d41897
                               at libstd/panicking.rs:211
   3:     0x7f381362c2c0 - std::panicking::default_hook::h3a27b204a313406b
                               at libstd/panicking.rs:227
   4:     0x7f380fe769f1 - rustc::util::common::panic_hook::hf9d72254884e6e21
   5:     0x7f381362cc53 - std::panicking::rust_panic_with_hook::h0e12cb2fc86d00fa
                               at libstd/panicking.rs:481
   6:     0x7f381362c7b9 - std::panicking::continue_panic_fmt::h141671b29fe0e27d
                               at libstd/panicking.rs:391
   7:     0x7f381362c6b5 - rust_begin_unwind
                               at libstd/panicking.rs:326
   8:     0x7f38136a55db - core::panicking::panic_fmt::h429a06507aba9228
                               at libcore/panicking.rs:77
   9:     0x7f38136a5671 - core::option::expect_failed::h4c79c3aae6612643
                               at libcore/option.rs:1000
  10:     0x7f38128de274 - rustc_resolve::macros::<impl syntax::ext::base::Resolver for rustc_resolve::Resolver<'a, 'crateloader>>::resolve_macro_invocation::hcacc2b568dda6de5
  11:     0x7f380eecdf28 - syntax::ext::expand::MacroExpander::expand_fragment::h26fd612375b32814
  12:     0x7f380eecd6b1 - syntax::ext::expand::MacroExpander::expand_crate::h3497aacb05bd8e53
  13:     0x7f38139c0303 - rustc_driver::driver::phase_2_configure_and_expand_inner::{{closure}}::h3d4d02fec2b68db2
  14:     0x7f38139bedda - rustc::util::common::time::ha1804c2290179406
  15:     0x7f38139d7b54 - rustc_driver::driver::phase_2_configure_and_expand::h2c60f5950dd2995f
  16:     0x7f38139d356b - rustc_driver::driver::compile_input::h6a302c45b9b8d410
  17:     0x7f3813a7b20c - rustc_driver::run_compiler_with_pool::h2a396394cdff52b0
  18:     0x7f38139d2f34 - rustc_driver::driver::spawn_thread_pool::h5dec0f6efcb9456b
  19:     0x7f3813a7a221 - rustc_driver::run_compiler::he4a6ce0f80da709f
  20:     0x7f38139c169c - <scoped_tls::ScopedKey<T>>::set::h0d2c211708f72196
  21:     0x7f38139fd651 - syntax::with_globals::hcc4912b6b554b6a2
  22:     0x7f381366c469 - __rust_maybe_catch_panic
                               at libpanic_unwind/lib.rs:103
  23:     0x7f3813a7847c - rustc_driver::run::h9e76da7d522d5bdd
  24:     0x7f3813a864ba - rustc_driver::main::h979d81f5800d15a6
  25:     0x55b562390992 - std::rt::lang_start::{{closure}}::h5cee4df267a41ddd
  26:     0x7f381362c652 - std::panicking::try::do_call::h572dc6f90deb1a4d
                               at libstd/rt.rs:59
                               at libstd/panicking.rs:310
  27:     0x7f381366c469 - __rust_maybe_catch_panic
                               at libpanic_unwind/lib.rs:103
  28:     0x7f3813642b45 - std::rt::lang_start_internal::hce01021c3c1cf20d
                               at libstd/panicking.rs:289
                               at libstd/panic.rs:392
                               at libstd/rt.rs:58
  29:     0x55b562390983 - main
  30:     0x7f3813400222 - __libc_start_main
  31:     0x55b562390838 - <unknown>
query stack during panic:
end of query stack

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: rustc 1.30.0 (da5f414c2 2018-10-24) running on x86_64-unknown-linux-gnu

rustc

$ rustc --version --verbose
rustc 1.30.0 (da5f414c2 2018-10-24)
binary: rustc
commit-hash: da5f414c2c0bfe5198934493f04c676e2b23ff2e
commit-date: 2018-10-24
host: x86_64-unknown-linux-gnu
release: 1.30.0
LLVM version: 8.0
@memoryruins memoryruins added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Oct 29, 2018
@awaitlink awaitlink changed the title Compiler panic (doc attribute, string concatenation) ❄️ in macro: doc meta with expr on an item, string concat, stringify!(...) Nov 3, 2018
@awaitlink awaitlink changed the title ❄️ in macro: doc meta with expr on an item, string concat, stringify!(...) ICE in macro: doc meta with expr on an item, string concat, stringify!(...) May 16, 2019
@jonas-schievink jonas-schievink added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Name resolution C-bug Category: This is a bug. I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 6, 2019
@jonas-schievink
Copy link
Contributor

Still reproduces on current stable and nightly

@nagisa nagisa added P-high High priority and removed I-nominated labels Aug 8, 2019
@nagisa
Copy link
Member

nagisa commented Aug 8, 2019

@petrochenkov this sounds like something you might want to look at.

@petrochenkov petrochenkov self-assigned this Aug 8, 2019
@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 30, 2019

Minimized further:

macro_rules! bug {
    ($expr:expr) => {
        #[rustc_dummy = $expr] // Any key-value attribute, not necessarily `doc`
        struct S;
    };
}

// Any expressions containing macro call `X` that's more complex than `X` itself.
// Parentheses will work.
bug!((line!()));

@petrochenkov
Copy link
Contributor

The ICE happens due to a mismatch in nonterminal token visiting between Visitor and MutVisitor (token::NtExpr in this case).

Visitor (BuildReducedGraphVisitor) doesn't visit them so macros in them don't get properly registered, MutVisitor (InvocationCollector) however visits them and attempts to expand macros in them even if they are not registered, so ICEs happen.

@petrochenkov petrochenkov added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Oct 10, 2019
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Oct 15, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Nov 14, 2019

@petrochenkov is the presumed fix here to add fn visit_interpolated and fn walk_interpolated to the syntax::visit::Visitor trait?

😆 we knew we needed to do that back in 2017: 9f1a8bf

@petrochenkov
Copy link
Contributor

That would fix the ICE, but

  • Visiting interpolated tokens as AST (mutably or not) was never a good idea. In the token-based macro expansion model (which is currently in effect in proc macros, but we'd like to migrate to it everywhere else) interpolated tokens contain tokens and not AST.
  • Make more code compile successfully despite us not wanting it to compile from the token-based model's point of view.

@petrochenkov
Copy link
Contributor

I have a plan on running some crater experiments to find out which kinds of expansions inside interpolated tokens are used in practice, but I need someone else's help to implement it due to lack of time.

@petrochenkov petrochenkov added the E-help-wanted Call for participation: Help is requested to fix this issue. label Nov 14, 2019
@petrochenkov
Copy link
Contributor

The experiment is to attempt restricting macro expansion inside attributes to #[attr = $expr].

Some relevant links are
#55414
#47358
#44732 (comment)

If the experiment goes well, then we'll proceed further in this direction and the end result should be an RFC about supporting macro expansions in key-value attributes like #[doc = include_str!("doc.md")].

Another link: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Nt.20tokens.20in.20the.20parser


So, the problem is that InvocationCollector collects and attempts to expand macro invocations inside tokens in attributes.

How does that happen?
Tokens in attributes can be nonterminal tokens (like NtExpr, aka $expr: expr in macro_rules in source code).
InvocationCollector is a MutVisitor and MutVisitor by default visit nonterminals as actual AST fragments (expressions, types, etc).
So, InvocationCollector visits all those fragments too, in any tokens, and attempts to expand them as macros.
The zulip link above shows how this can be used for building the value of a #[doc = ...] attribute with a macro.

We need to restrict InvocationCollector to only doing this when the attribute looks like this - #[attr = $expr] (not #[attr(1 2 $expr 3 4)] or anything else).
This can be done by overriding fn visit_attribute, detecting key-value attributes in it, visiting only them, and not recursing further into tokens.

@petrochenkov
Copy link
Contributor

It's simpler to implement this on top of #66935.

@CAD97
Copy link
Contributor

CAD97 commented Jan 10, 2020

I just ran into this with a doc_comment! macro:

doc_comment! {
    concat("Create a new ", stringify!($RcBox), "."),
    pub fn new(data: T) -> Self
    where
        T: Sized,
    {
        unsafe { $RcBox::from_unchecked($Rc::new(data)) }
    }
}

The typo of concat( instead of concat!( lead to this ICE.

This does show the common "abuse" of #[doc = $expr] and how easily it can go wrong, however.

@spastorino
Copy link
Member

As per Zulip discussion changing priority to P-medium

@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 27, 2020

I started working on this in #77271.
UPD: The work is completed.

@bors bors closed this as completed in 4c0c5e0 Nov 3, 2020
jyn514 added a commit to jyn514/rust that referenced this issue May 18, 2021
 # Stabilization report

 ## Summary

This stabilizes using macro expansion in key-value attributes, like so:

 ```rust
 #[doc = include_str!("my_doc.md")]
 struct S;

 #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
 mod m;
 ```

See the changes to the reference for details on what macros are allowed;
see Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455)
for alternatives that were considered and rejected ("why accept no more
and no less?")

This has been available on nightly since 1.50 with no major issues.

 ## Notes

 ### Accepted syntax

The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`).  Note that decorators and the like may be able to observe other expression forms.

 ### Expansion ordering

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

 ## Test cases

 - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs
 - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs

The feature has also been dogfooded extensively in the compiler and
standard library:

- rust-lang#83329
- rust-lang#83230
- rust-lang#82641
- rust-lang#80534

 ## Implementation history

- Initial proposal: rust-lang#55414 (comment)
- Experiment to see how much code it would break: rust-lang#67121
- Preliminary work to restrict expansion that would conflict with this
feature: rust-lang#77271
- Initial implementation: rust-lang#78837
- Fix for an ICE: rust-lang#80563

 ## Unresolved Questions

~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~

 ## Additional Information

 There are two workarounds that have a similar effect for `#[doc]`
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:

```rust
macro_rules! forward_inner_docs {
    ($e:expr => $i:item) => {
        #[doc = $e]
        $i
    };
}

forward_inner_docs!(include_str!("lib.rs") => struct S {});
```

This also works for other attributes (like `#[path = concat!(...)]`).
The other is to use `doc(include)`:

```rust
 #![feature(external_doc)]
 #[doc(include = "lib.rs")]
 struct S {}
```

The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.

I plan to remove `doc(include)` when this is stabilized. The
`forward_inner_docs` workaround will still compile without warnings, but
I expect it to be used less once it's no longer necessary.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 18, 2021
…=petrochenkov

Stabilize extended_key_value_attributes

Closes rust-lang#44732. Closes rust-lang#78835. Closes rust-lang#82768 (by making it irrelevant).

 # Stabilization report

 ## Summary

This stabilizes using macro expansion in key-value attributes, like so:

 ```rust
 #[doc = include_str!("my_doc.md")]
 struct S;

 #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
 mod m;
 ```

See Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455)
for alternatives that were considered and rejected ("why accept no more and no less?")

This has been available on nightly since 1.50 with no major issues.

## Notes

### Accepted syntax

The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`).  Note that decorators and the like may be able to observe other expression forms.

### Expansion ordering

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

## Test cases

 - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs
 - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs

The feature has also been dogfooded extensively in the compiler and
standard library:

- rust-lang#83329
- rust-lang#83230
- rust-lang#82641
- rust-lang#80534

## Implementation history

- Initial proposal: rust-lang#55414 (comment)
- Experiment to see how much code it would break: rust-lang#67121
- Preliminary work to restrict expansion that would conflict with this
feature: rust-lang#77271
- Initial implementation: rust-lang#78837
- Fix for an ICE: rust-lang#80563

## Unresolved Questions

~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~

 ## Additional Information

 There are two workarounds that have a similar effect for `#[doc]`
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:

```rust
macro_rules! forward_inner_docs {
    ($e:expr => $i:item) => {
        #[doc = $e]
        $i
    };
}

forward_inner_docs!(include_str!("lib.rs") => struct S {});
```

This also works for other attributes (like `#[path = concat!(...)]`).
The other is to use `doc(include)`:

```rust
 #![feature(external_doc)]
 #[doc(include = "lib.rs")]
 struct S {}
```

The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.

I plan to remove `doc(include)` when this is stabilized
(rust-lang#82539). The `forward_inner_docs`
workaround will still compile without warnings, but I expect it to be
used less once it's no longer necessary.
jackh726 added a commit to jackh726/rust that referenced this issue May 19, 2021
…=petrochenkov

Stabilize extended_key_value_attributes

Closes rust-lang#44732. Closes rust-lang#78835. Closes rust-lang#82768 (by making it irrelevant).

 # Stabilization report

 ## Summary

This stabilizes using macro expansion in key-value attributes, like so:

 ```rust
 #[doc = include_str!("my_doc.md")]
 struct S;

 #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
 mod m;
 ```

See Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455)
for alternatives that were considered and rejected ("why accept no more and no less?")

This has been available on nightly since 1.50 with no major issues.

## Notes

### Accepted syntax

The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`).  Note that decorators and the like may be able to observe other expression forms.

### Expansion ordering

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

## Test cases

 - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs
 - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs

The feature has also been dogfooded extensively in the compiler and
standard library:

- rust-lang#83329
- rust-lang#83230
- rust-lang#82641
- rust-lang#80534

## Implementation history

- Initial proposal: rust-lang#55414 (comment)
- Experiment to see how much code it would break: rust-lang#67121
- Preliminary work to restrict expansion that would conflict with this
feature: rust-lang#77271
- Initial implementation: rust-lang#78837
- Fix for an ICE: rust-lang#80563

## Unresolved Questions

~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~

 ## Additional Information

 There are two workarounds that have a similar effect for `#[doc]`
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:

```rust
macro_rules! forward_inner_docs {
    ($e:expr => $i:item) => {
        #[doc = $e]
        $i
    };
}

forward_inner_docs!(include_str!("lib.rs") => struct S {});
```

This also works for other attributes (like `#[path = concat!(...)]`).
The other is to use `doc(include)`:

```rust
 #![feature(external_doc)]
 #[doc(include = "lib.rs")]
 struct S {}
```

The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.

I plan to remove `doc(include)` when this is stabilized
(rust-lang#82539). The `forward_inner_docs`
workaround will still compile without warnings, but I expect it to be
used less once it's no longer necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Name resolution C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants