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

Count chars when creating SourceAnnotation #6084

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MarcusGrass
Copy link
Contributor

@MarcusGrass MarcusGrass commented Feb 21, 2024

Fixes #6083.

In annotate snippets it takes the length of the string by char count https://github.com/rust-lang/annotate-snippets-rs/blob/master/src/renderer/display_list.rs#L935, both on 0.9 and 0.10, but in the code here the
length is taken by byte-index.

Since the user's string has chars that do not correspond 1-to-1 with byte-indices there's a range-mismatch and a following panic.

It only occurs in some cases and was fairly hard to reproduce, I think it has something to do with max line-count as well, and that's when it blows up.

I'm not so familiar with this repo, I can't get the test to pass. The source can't be formatted, since the comment would be lost, but if that's disabled rustfmt will err on trailing whitespace and I don't know if that can be disabled, but the test really checks that rustfmt doesn't panic, so maybe it just doesn't fit into this mould.
E: I put the test-file under its own directory and added it to the dont_emit_ice_test instead.

Can easily be reproduced by running rustfmt tests/issue-6083/issue_6083.rs vs cargo run --bin rustfmt -- tests/issue-6083/issue_6083.rs on this branch.

@ytmimi
Copy link
Contributor

ytmimi commented Feb 21, 2024

@MarcusGrass thanks for looking into this! I came to the same conclusion regarding chars vs bytes when I looked into this a while ago. I'll follow up when I've had a chance to take a look at this / refresh my memory on what I was looking into previously. This has been a long standing issue and I'd be very glad if we could fix it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] panicked at SourceAnnotation range (91, 92) is bigger than source length 69
3 participants