-
Notifications
You must be signed in to change notification settings - Fork 925
panic in annotated_snippet dep #4968
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
Comments
Could you elaborate on what you're trying to do? Not going to pretend like the panic is desirable because to the best of my knowledge we don't have any test cases set up to intentionally trigger any. However, folks should never run any version of rustfmt directly against anything under |
Ah well, I was running rustfmt on all the |
Ref rust-lang#4968. Tabs were always counted to have a width of config.tab_spaces. Since they actually can have less than that depending on the line's previous contents, rustfmt would sometimes complain about the line length even if the line did actually fit.
Ref rust-lang#4968. Tabs were always counted to have a width of config.tab_spaces. Since they actually can have less than that depending on the line's previous contents, rustfmt would sometimes complain about the line length even if the line did actually fit.
I reduced the panic to this:
The problem is that rustfmt treats counts tabs as multiple characters (code), but annotate_snippets counts tabs as one character. So if rustfmt produces an error at the end of a line that contains tabs, annotate_snippets thinks that error is out of range. There's a somewhat related bug there: rust-lang/annotate-snippets-rs#25. Something similar would probably happen when using tabs for indentation. I don't know how to fix this. In addition, rustfmt always gave tabs a fixed width, even though they might be narrower depending on the line's previous contents. This made rustfmt complain about lines that actually fit into the line limit. I fixed this in #5039. |
I'm currently running into this problem. This seems to have stalled. When is #5039 going to be merged? |
* Internally, rustfmt preserves tabs and counts them as multiple characters (based on configuration). * The annoted_snippet dependency counts tabs as 1 character. * If rustfmt produces an error on a line containing tabs, annotated_snippet may think that the error is out of range and panic. * Have rustfmt internally replace tabs with the corresponding number of spaces, so that columns can be counted unambiguously. * This change is based on PR rust-lang#5039 by karyon, but with the code review suggestions by camsteffen.
I also stumbled across a similar panic, it's probably a similar bug, since it involves a wide unicode character In the rustfmt_crash.rs file:
Note that there are 2 spaces after
|
* Internally, rustfmt preserves tabs and counts them as multiple characters (based on configuration). * The annotated_snippet dependency always counts tabs as 1 character. * If rustfmt tries to display an error on a line containing tabs, the indicies are mismatched. * In the extreme case, annotated_snippet may try to access out-of-range indices, and panic. * This change is based on the code review by camsteffen on PR rust-lang#5039 by karyon: have rustfmt internally replace tabs with the corresponding number of spaces, so that columns/indices in the buffer passed to annotated_snippet are counted (unambiguously and) the same.
* Internally, rustfmt preserves tabs and counts them as multiple characters (based on configuration). * The annotated_snippet dependency always counts tabs as 1 character. * If rustfmt tries to display an error on a line containing tabs, the indicies are mismatched. * In the extreme case, annotated_snippet may try to access out-of-range indices, and panic. * This change is based on the code review by camsteffen on PR rust-lang#5039 by karyon: have rustfmt internally replace tabs with the corresponding number of spaces, so that columns/indices in the buffer passed to annotated_snippet are counted (unambiguously and) the same.
* With hard_tabs, rustfmt preserves tabs and counts them as multiple characters/columns (based on configuration). * The annotated_snippet dependency, used to display errors, always counts tabs as 1 character. * If rustfmt tries to report an error on a line containing tabs, the indices are mismatched. * annotated_snippet will display the wrong range of the source code slice; in the extreme case, it can panic with out-of-bounds access. * The test case added in this commit is expected to currently fail, since this commit doesn't include the fix.
* This change is based on the code review by camsteffen on PR rust-lang#5039 by karyon: have rustfmt internally replace tabs with the corresponding number of spaces, so that columns/indices in the buffer passed to annotated_snippet are counted unambiguously.
* This change is based on the code review by camsteffen on PR rust-lang#5039 by karyon: have rustfmt internally replace tabs with the corresponding number of spaces, so that columns/indices in the buffer passed to annotated_snippet are counted unambiguously.
* With hard_tabs, rustfmt preserves tabs and counts them as multiple characters/columns (based on configuration). * The annotated_snippet dependency, used to display errors, always counts tabs as 1 character. * If rustfmt tries to report an error on a line containing tabs, the indices are mismatched. * annotated_snippet will display the wrong range of the source code slice; in the extreme case, it can panic with out-of-bounds access. * The test case added in this commit is expected to currently fail, since this commit doesn't include the fix.
* With hard_tabs, rustfmt preserves tabs and counts them as multiple characters/columns (based on configuration). * The annotated_snippet dependency, used to display errors, always counts tabs as 1 character. * If rustfmt tries to report an error on a line containing tabs, the indices are mismatched. * annotated_snippet will display the wrong range of the source code slice; in the extreme case, it can panic with out-of-bounds access. * The test case added in this commit is expected to currently fail, since this commit doesn't include the fix.
When I run
rustfmt ./src/tools/rustfmt/tests/source/cfg_if/mod.rs
inside the rustc repo, I get a panic inside theannotated-snipped
dependency. Note: it seems that the version that rustfmt uses is outdated (0.8.0 vs 0.9.0) but I don't know if a dependency update will fix the crash.code:
stacktrace:
The text was updated successfully, but these errors were encountered: