rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

Panic in SourceAnnotation when using `hard_tabs = true`

Open lbfalvy opened this issue 10 months ago • 13 comments

Here's the code it died on

https://github.com/lbfalvy/orchid/tree/e0d246fe1fa75405305ec4390ba6815049246726

Logfile the panic asked me to attach

rustc-ice-2025-01-12T01_00_58-10688.txt

lbfalvy avatar Jan 12 '25 01:01 lbfalvy

I tried a freshly installed nightly toolchain on Ubuntu and got the same error. Same codebase, here's the crash report:

rustc-ice-2025-01-12T17_57_36-6548.txt

lbfalvy avatar Jan 12 '25 17:01 lbfalvy

@lbfalvy can you please get this down to a minimal example that can be used to reproduce the error. Also, it would be good to know the exact version of rustfmt that you're using and if you're using any non-default rustfmt configs.

ytmimi avatar Jan 13 '25 00:01 ytmimi

@ytmimi All done, it's very similar to other SourceAnnotation bugs but I can't find the specific issue that had a really long print statement as a minimal example.

For completeness' sake, here's my minimal example.

repo: https://github.com/lbfalvy/orchid/tree/rustfmt-minimal-crash

It's just one file:

// src/main.rs
pub fn main() {
	println!(
		"Reeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"
	)
}

rustfmt options:

# rustfmt.toml
unstable_features = true
style_edition = "2024"

hard_tabs = true
# the following two are unstable
error_on_line_overflow = true
error_on_unformatted = true

I checked, all three options are necessary to trigger the panic, any other combination results in either a successful run or an explicit formatting error.

rustfmt version according to cargo fmt -- --version:

rustfmt 1.8.0-nightly (48a426eca9 2025-01-12)

And finally the crash file

rustc-ice-2025-01-13T14_58_18-35820.txt

lbfalvy avatar Jan 13 '25 15:01 lbfalvy

Thank you for the extra info!

ytmimi avatar Jan 13 '25 16:01 ytmimi

Because this issue involves tabs, I think this is related to https://github.com/rust-lang/rustfmt/issues/4968#issuecomment-949461340

ytmimi avatar Jan 13 '25 16:01 ytmimi

Just checked with main because I saw that #6391 got merged and it seems to be working fine. Thank you!

lbfalvy avatar Jan 15 '25 22:01 lbfalvy

From local testing with rustfmt 1.8.0-nightly (d97c4fb3e2 2025-01-13) this snippet from https://github.com/rust-lang/rustfmt/issues/6442#issuecomment-2587385011 still panics with hard_tabs=true

ytmimi avatar Jan 16 '25 17:01 ytmimi

@ytmimi right, then it looks like my minimal example wasn't accurate because the original full project now correctly reports an error instead of panicking.

lbfalvy avatar Jan 16 '25 20:01 lbfalvy

I see. Either way, the MCVE you provided still panics, and I think that's a good enough reason to keep the issue open

ytmimi avatar Jan 16 '25 20:01 ytmimi

I'm affected, too. This issue seems to be caused by rustfmt tracking line widths and positions in character width units. Normally this isn't too obvious, but if hard_tabs is true, a tab character may actually be tracked as a width of 4 or 8, for example. annotate-snippets expects offsets in characters relative to the start of the source() parameter. Thus, formatting ErrorKind::LineOverflow will always panic if there is a leading tab indent.

naseschwarz avatar Mar 11 '25 15:03 naseschwarz

Just to clarify annotate-snippets used to take character offsets. As of a more recent release it uses byte offsets. I Agree with your assessment of the underlying issue though.

ytmimi avatar Mar 11 '25 16:03 ytmimi

I may have a practical solution that avoids messing too much with hard_tabs in #6500. Instead of trying to do index magic, I'd suggest to just report errors with lines converted to leading space representation. See my reasoning in the PR.

Thanks for the notice about byte offsets. I guess #6500 won't break (or fix) anything regarding this change.

~~This is still a draft, I'll add a test, too.~~ (Not sure where, to be honest...)

naseschwarz avatar Mar 11 '25 16:03 naseschwarz

Also, here's a minimum example ready to clone: https://github.com/naseschwarz/rustfmt-issue-6442

naseschwarz avatar Mar 11 '25 17:03 naseschwarz