rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
6.01k stars 883 forks source link

ICE in special `cfg_if` handling #5413

Open mpfaff opened 2 years ago

mpfaff commented 2 years ago

If you invoke any macro named cfg_if with starting with if and not following the rules of the macro of the same name provided by the cfg-if crate, rustfmt will panic.

Here is a minimal example to reproduce the issue:

cfg_if! {
    if
}

Output when running RUST_BACKTRACE=1 rustfmt +stable lib.rs:

error: internal compiler error: the following error was constructed but not emitted

error: expected `#`, found `<eof>`
 --> lib.rs:2:5
  |
2 |     if
  |     ^^

thread 'main' panicked at 'explicit panic', compiler/rustc_errors/src/diagnostic_builder.rs:553:21
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::panicking::panic
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:48:5
   3: <rustc_errors::diagnostic_builder::DiagnosticBuilderInner as core::ops::drop::Drop>::drop
   4: rustfmt_nightly::parse::macros::cfg_if::parse_cfg_if_inner
   5: <rustfmt_nightly::modules::visitor::CfgIfVisitor as rustc_ast::visit::Visitor>::visit_mac_call
   6: rustc_ast::visit::walk_item::<rustfmt_nightly::modules::visitor::CfgIfVisitor>
   7: <rustfmt_nightly::modules::ModResolver>::visit_cfg_if
   8: <rustfmt_nightly::modules::ModResolver>::visit_mod_from_ast
   9: <rustfmt_nightly::modules::ModResolver>::visit_crate
  10: rustfmt_nightly::formatting::format_project::<rustfmt_nightly::Session<std::io::stdio::Stdout>>
  11: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::with::<<rustfmt_nightly::Session<std::io::stdio::Stdout>>::format_input_inner::{closure#0}, core::result::Result<rustfmt_nightly::FormatReport, rustfmt_nightly::ErrorKind>>
  12: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_span::create_session_if_not_set_then<core::result::Result<rustfmt_nightly::FormatReport, rustfmt_nightly::ErrorKind>, <rustfmt_nightly::Session<std::io::stdio::Stdout>>::format_input_inner::{closure#0}>::{closure#0}, core::result::Result<rustfmt_nightly::FormatReport, rustfmt_nightly::ErrorKind>>
  13: <rustfmt_nightly::Session<std::io::stdio::Stdout>>::format
  14: rustfmt::format_and_emit_report::<std::io::stdio::Stdout>
  15: <rustfmt_nightly::Session<std::io::stdio::Stdout>>::override_config::<rustfmt::format::{closure#0}, ()>
  16: rustfmt::execute
  17: rustfmt::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

And when running RUST_BACKTRACE=1 rustfmt +nightly lib.rs:

error: internal compiler error: the following error was constructed but not emitted

error: expected `#`, found `<eof>`
 --> lib.rs:2:5
  |
2 |     if
  |     ^^

thread 'main' panicked at 'explicit panic', compiler/rustc_errors/src/diagnostic_builder.rs:568:21
stack backtrace:
   0: rust_begin_unwind
             at /rustc/2f3ddd9f594adf9773547aa7cedb43c4ac8ffd2f/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/2f3ddd9f594adf9773547aa7cedb43c4ac8ffd2f/library/core/src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/2f3ddd9f594adf9773547aa7cedb43c4ac8ffd2f/library/core/src/panicking.rs:48:5
   3: <rustc_errors::diagnostic_builder::DiagnosticBuilderInner as core::ops::drop::Drop>::drop
   4: rustfmt_nightly::parse::macros::cfg_if::parse_cfg_if_inner
   5: <rustfmt_nightly::modules::visitor::CfgIfVisitor as rustc_ast::visit::Visitor>::visit_mac_call
   6: rustc_ast::visit::walk_item::<rustfmt_nightly::modules::visitor::CfgIfVisitor>
   7: <rustfmt_nightly::modules::ModResolver>::visit_cfg_if
   8: <rustfmt_nightly::modules::ModResolver>::visit_mod_from_ast
   9: <rustfmt_nightly::modules::ModResolver>::visit_crate
  10: rustfmt_nightly::formatting::format_project::<rustfmt_nightly::Session<std::io::stdio::Stdout>>
  11: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::with::<<rustfmt_nightly::Session<std::io::stdio::Stdout>>::format_input_inner::{closure#0}, core::result::Result<rustfmt_nightly::FormatReport, rustfmt_nightly::ErrorKind>>
  12: <rustfmt_nightly::Session<std::io::stdio::Stdout>>::format
  13: rustfmt::format_and_emit_report::<std::io::stdio::Stdout>
  14: <rustfmt_nightly::Session<std::io::stdio::Stdout>>::override_config::<rustfmt::format::{closure#0}, ()>
  15: rustfmt::execute
  16: rustfmt::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Line 7 of each backtrace hints that special support for the cfg_if macro is the culprit.

Update 1:

This can also be triggered by else if, but reproducing it is a bit trickier, because it needs to avoid panicking on the first if:

cfg_if! {
    if #[_] { } else if
}

Update 2:

I can file a separate issue for this if that would be best, but for now I'll just tack it on here:

The special cfg_if handling can also deadlock as seen here:

cfg_if! {
    if #[_] { if }
}
ytmimi commented 2 years ago

Thanks for the report!

This is definitely something we'll want to take a look at. I also want to point out that the deadlock case is a duplicate of #4442

mpfaff commented 2 years ago

I also want to point out that the deadlock case is a duplicate of #4442

Thanks for letting me know. I posted my example over there.

calebcartwright commented 2 years ago

I appreciate the report @mpfaff and for your investigation @ytmimi, but want to be fully transparent that this will be a low priority.

There's very few macro calls which we provide special case handling for, and only do so in those cases because of a known, minimal set of arg tokens to expect, such as cfg_if. While it would of course be better to handle this more gracefully, handling intentionally incorrect/invalid call args gracefully isn't a high priority compared to the myriad of other items on the backlog.

ytmimi commented 2 years ago

Another cfg_if related issue popped up (#5428) so I started digging back into this. Although, diagnostic information is emitted, the invalid cfg_if syntax here doesn't actually prevent the file from being formatted as demonstrated by the following example:

Input:

cfg_if! {
    if
}

fn     main(
) {
println!("hello world");
}

Output

cfg_if! {
    if
}

fn main() {
    println!("hello world");
}