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 when formatting trait declaration with non-NFC name #6069

Closed
Jules-Bertholet opened this issue Feb 9, 2024 · 3 comments · Fixed by #6073
Closed

ICE when formatting trait declaration with non-NFC name #6069

Jules-Bertholet opened this issue Feb 9, 2024 · 3 comments · Fixed by #6073
Labels
bug Panic, non-idempotency, invalid code, etc. good first issue Issues up for grabs, also good candidates for new rustfmt contributors

Comments

@Jules-Bertholet
Copy link
Contributor

trait Foó: Bar {}

The above is two codepoints, ASCII o followed by U+0301 COMBINING ACUTE ACCENT. It NFC-normalizes to ó, U+00F3 LATIN SMALL LETTER O WITH ACUTE.

Trying to format the above results in:

thread 'main' panicked at src/tools/rustfmt/src/source_map.rs:31:13:
bad span: `Foó`: `trait Foó: Bar {}`
stack backtrace:
   0:     0x7f3c6b9a06f6 - std::backtrace_rs::backtrace::libunwind::trace::hbee8a7973eeb6c93
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:     0x7f3c6b9a06f6 - std::backtrace_rs::backtrace::trace_unsynchronized::hc8ac75eea3aa6899
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7f3c6b9a06f6 - std::sys_common::backtrace::_print_fmt::hc7f3e3b5298b1083
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x7f3c6b9a06f6 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hbb235daedd7c6190
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x7f3c6b9f2f40 - core::fmt::rt::Argument::fmt::h76c38a80d925a410
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/rt.rs:142:9
   5:     0x7f3c6b9f2f40 - core::fmt::write::h3ed6aeaa977c8e45
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/fmt/mod.rs:1120:17
   6:     0x7f3c6b99453f - std::io::Write::write_fmt::h78b18af5775fedb5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/io/mod.rs:1810:15
   7:     0x7f3c6b9a04d4 - std::sys_common::backtrace::_print::h5d645a07e0fcfdbb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x7f3c6b9a04d4 - std::sys_common::backtrace::print::h85035a511aafe7a8
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x7f3c6b9a3267 - std::panicking::default_hook::{{closure}}::hcce8cea212785a25
  10:     0x7f3c6b9a2fc9 - std::panicking::default_hook::hf5fcb0f213fe709a
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:292:9
  11:     0x7f3c6e62961c - std[79729d9c385e1623]::panicking::update_hook::<alloc[6df67106ebca92c0]::boxed::Box<rustc_driver_impl[66ed8fdbde15dc6c]::install_ice_hook::{closure#0}>>::{closure#0}
  12:     0x7f3c6b9a39b6 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::hbc5ccf4eb663e1e5
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2029:9
  13:     0x7f3c6b9a39b6 - std::panicking::rust_panic_with_hook::h095fccf1dc9379ee
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:783:13
  14:     0x7f3c6b9a3702 - std::panicking::begin_panic_handler::{{closure}}::h032ba12139b353db
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:657:13
  15:     0x7f3c6b9a0bf6 - std::sys_common::backtrace::__rust_end_short_backtrace::h9259bc2ff8fd0f76
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:171:18
  16:     0x7f3c6b9a3460 - rust_begin_unwind
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
  17:     0x7f3c6b9ef645 - core::panicking::panic_fmt::h784f20a50eaab275
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
  18:     0x55e4891a72ac - <rustfmt_nightly[9b2d04b1e1d23002]::visitor::SnippetProvider as rustfmt_nightly[9b2d04b1e1d23002]::source_map::SpanUtils>::span_after
  19:     0x55e4891bee27 - <rustfmt_nightly[9b2d04b1e1d23002]::visitor::FmtVisitor>::visit_item
  20:     0x55e48919f146 - <rustfmt_nightly[9b2d04b1e1d23002]::visitor::FmtVisitor>::visit_items_with_reordering
  21:     0x55e4891c6e7b - <rustfmt_nightly[9b2d04b1e1d23002]::visitor::FmtVisitor>::format_separate_mod
  22:     0x55e489038a85 - <rustfmt_nightly[9b2d04b1e1d23002]::Session<std[79729d9c385e1623]::io::stdio::Stdout>>::format_input_inner::{closure#0}
  23:     0x55e48904d3c0 - rustfmt[c449a577f49ce32d]::format_and_emit_report::<std[79729d9c385e1623]::io::stdio::Stdout>
  24:     0x55e48904bc5e - rustfmt[c449a577f49ce32d]::execute
  25:     0x55e4890474c6 - rustfmt[c449a577f49ce32d]::main
  26:     0x55e489032533 - std[79729d9c385e1623]::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>
  27:     0x55e489034749 - std[79729d9c385e1623]::rt::lang_start::<()>::{closure#0}
  28:     0x7f3c6b984cb1 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h37600b1e5eea4ecd
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:284:13
  29:     0x7f3c6b984cb1 - std::panicking::try::do_call::hb4bda49fa13a0c2b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  30:     0x7f3c6b984cb1 - std::panicking::try::h8bbf75149211aaaa
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  31:     0x7f3c6b984cb1 - std::panic::catch_unwind::h8c78ec68ebea34cb
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  32:     0x7f3c6b984cb1 - std::rt::lang_start_internal::{{closure}}::hffdf44a19fd9e220
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:48
  33:     0x7f3c6b984cb1 - std::panicking::try::do_call::hcb3194972c74716d
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  34:     0x7f3c6b984cb1 - std::panicking::try::hcdc6892c5f0dba4c
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  35:     0x7f3c6b984cb1 - std::panic::catch_unwind::h4910beb4573f4776
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  36:     0x7f3c6b984cb1 - std::rt::lang_start_internal::h6939038e2873596b
                               at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:20
  37:     0x55e48904e5d5 - main
  38:     0x7f3c6b590083 - __libc_start_main
  39:     0x55e4890212c9 - <unknown>
  40:                0x0 - <unknown>

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

note: we would appreciate a bug report: https:/rust-lang/rustfmt/issues/new?labels=bug

note: rustc 1.76.0 (07dca489a 2024-02-04) running on x86_64-unknown-linux-gnu

query stack during panic:
end of query stack
@ytmimi
Copy link
Contributor

ytmimi commented Feb 13, 2024

@Jules-Bertholet you mentioned back in #6058 that the compiler is now normalizing idents? Has that been a thing for a while now? Is there a way to get the unnormalized ident? The only way I can think to do so is to use the ident's span.

Let me know if I'm understanding this one correctly. I believe what you mentioned above is that Foó above is actually Foo\u{301}, which is getting interpreted as Foó. At least from looking into this it seems that calling item.ident.as_str() returns Foó. From what I can tell this is causing issues because rustfmt is now trying to find Foó in trait Foo\u{301}: Bar {}, and panics! when it can't do so.

This is where the panic happens:

rustfmt/src/items.rs

Lines 1169 to 1171 in 8486837

let ident_hi = context
.snippet_provider
.span_after(item.span, item.ident.as_str());

@ytmimi ytmimi added the bug Panic, non-idempotency, invalid code, etc. label Feb 13, 2024
@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented Feb 13, 2024

Let me know if I'm understanding this one correctly

@ytmimi You are.

Has that been a thing for a while now?

Identifier normalization has been a thing since Rust started supporting non-ASCII idents, changing normalization behavior would have been a breaking change (it would change whether two identifiers are considered to refer to the same thing). I can't speak to the history of rustc's exact implementation though

@ytmimi
Copy link
Contributor

ytmimi commented Feb 13, 2024

Cool! This should be a straightforward fix. As mentioned, this is where the panic originates:

rustfmt/src/items.rs

Lines 1169 to 1171 in 8486837

let ident_hi = context
.snippet_provider
.span_after(item.span, item.ident.as_str());

And I think the fix here is to ensure we're using the right needle when searching for the end of the ident:

+        // Idents are NFC normalized by the compiler. To make sure we provide the right needle to
+        // `span_after` we grab the ident from the source file. See #6069
+        let source_ident = context.snippet(item.ident.span);
         let ident_hi = context
             .snippet_provider
-            .span_after(item.span, item.ident.as_str());
+            .span_after(item.span, source_ident);

Then we should add a test case with the snippet posted above to a file like tests/target/issue_6069.rs, and to further test that the panic is resolved we can add tests/target/issue_6069.rs to this list:

#[test]
#[allow(non_snake_case)]
fn dont_emit_ICE() {
let files = ["tests/target/issue_5728.rs", "tests/target/issue_5729.rs"];
for file in files {
let args = [file];
let (_stdout, stderr) = rustfmt(&args);
assert!(!stderr.contains("thread 'main' panicked"));
}
}

@ytmimi ytmimi added the good first issue Issues up for grabs, also good candidates for new rustfmt contributors label Feb 13, 2024
Jules-Bertholet added a commit to Jules-Bertholet/rustfmt that referenced this issue Feb 13, 2024
Jules-Bertholet added a commit to Jules-Bertholet/rustfmt that referenced this issue Feb 13, 2024
Jules-Bertholet added a commit to Jules-Bertholet/rustfmt that referenced this issue Feb 14, 2024
Jules-Bertholet added a commit to Jules-Bertholet/rustfmt that referenced this issue Feb 14, 2024
Jules-Bertholet added a commit to Jules-Bertholet/rustfmt that referenced this issue Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Panic, non-idempotency, invalid code, etc. good first issue Issues up for grabs, also good candidates for new rustfmt contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants