Skip to content

Bare CR allowed in code causing terrible spans #16480

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

Closed
SiegeLord opened this issue Aug 13, 2014 · 6 comments
Closed

Bare CR allowed in code causing terrible spans #16480

SiegeLord opened this issue Aug 13, 2014 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-grammar Area: The grammar of Rust C-bug Category: This is a bug. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@SiegeLord
Copy link
Contributor

E.g. consider this code (save it to a file with CR line endings):

fn main() {
    let a = 0;
}

Error:

test.rs:1:18: 1:23 error: cannot determine a type for this local variable: cannot determine the type of this integer; add a suffix to specify the type explicitly [E0102]
}est.rs:let a = 0;) {
                            ^~~~~
error: aborting due to previous error

I'm pretty sure this did not used to happen.

I suggest disallowing bare CRs rather than fixing this span issue.

@lilyball
Copy link
Contributor

Two options:

  1. Disallow bare CRs everywhere, not just in string literals.
  2. Translate bare CRs into LFs outside of string literals (still disallow them inside).

Said translation would have to be done when printing the source and when determining where line breaks are (which I assume the span line has to do), because AFAIK the spans point into the original immutable source. If the source is actually mutable in memory, then you could replace bare CR with LF there and call it a day, but I'm guessing that's not feasible (I've never looked into the file map).

@SiegeLord
Copy link
Contributor Author

Actually, CR isn't the only character that breaks error spans. E.g. a stray LS (U+2028) also leads to some span wonkiness. A non-committal solution would be to let things be from the language specification standpoint, and just replace all of these wonky whitespace characters with spaces for the purposes of error reporting.

@steveklabnik steveklabnik added the A-grammar Area: The grammar of Rust label Jan 27, 2015
@steveklabnik
Copy link
Member

Triage: no change.

@Mark-Simulacrum Mark-Simulacrum added A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. labels Jul 21, 2017
@estebank estebank added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority labels May 25, 2019
@jonas-schievink jonas-schievink added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 12, 2020
@klensy
Copy link
Contributor

klensy commented Jan 31, 2021

Still the same:
main.rs with crlf or lf endings:

fn main() {
    let a = 0;
}
$ rustc main.rs
warning: unused variable: `a`
 --> main.rs:2:9
  |
2 |     let a = 0;
  |         ^ help: if this is intentional, prefix it with an underscore: `_a`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

same, main.rs, but with cr endings:

$ rustc main.rs
warning: unused variable: `a`
 --> main.rs:1:21
  |
}   let a = 0;{
  |                    ^ help: if this is intentional, prefix it with an underscore: `_a`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

@estebank
Copy link
Contributor

Since #90462 we have replacement machinery for arbitrary codepoints that we could use for this. At the very least it would be easy to replace these with (if replacement with \n proves to require more extensive changes) to prevent further presentation issues quickly. At the same time, we should have a warn-by-default lint for these codepoints so that we can emit a targeted message about these codepoints with applicable suggestions.

matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this issue Feb 11, 2024
…lnicola

Remove unused Subtree::visit_ids function

Completely unused, nothing else even shows up when searching for `visit_ids`.
@estebank
Copy link
Contributor

This was fixed by #127528.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-grammar Area: The grammar of Rust C-bug Category: This is a bug. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants