-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
fix(lexer): use is_asscii_whitespace
to find first not space pos
#108403
Conversation
r? @eholk (rustbot has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think #108275 is about the diagnostic output and not the fact that not all ascii whitespace characters are skipped. Are we sure we want to skip more characters?
Maybe the |
This change looks reasonable to me, but I'd love to get someone from @rust-lang/lang to weigh in. It's not clear whether we've implemented the behavior we want and described it imprecisely, or described the behavior we want but implemented it incorrectly. I'd lean towards the latter (and that's the position this PR takes), but I don't feel qualified to declare that on my own. |
Seeing that it is the code implemented in #60261, perhaps we can ask @matklad or @petrochenkov to give some advice |
My gut feeling is that only the error message is wrong here. It might be argued that the language definition is wrong here, and that we should adjust that. It indeed feels wrong that we use different definitions of whitespace when lexing tokens and when processing innards of a string. But, given that changing this would be a backwards incompatible change, I am 0.7 sure that the outcome here would be "this is a documented peculiarity". |
FWIW, the high order bit here is that this changes the visible semantics of the language. At this point in Rust's lifetime, "changes behavior" should take precedence over "reasonableness". |
Very great explanation, I will give a PR with only the error message modified in the near future |
Closing, since the consensus seems to be that we only need to change the diagnostic. |
fix(lexer): print whitespace warning for \x0c - close rust-lang#108275 - discussion: rust-lang#108403
close #108275