Improve raw string diagnostic #61255
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
hellow554
force-pushed
the
raw_string_diag
branch
from
f8d3b2d
to
541d42e
Compare
petervaro
reviewed
May 28, 2019
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
hellow554
force-pushed
the
raw_string_diag
branch
from
541d42e
to
6e90f7f
Compare
|
cc @matklad |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Should I rebase the 7 commits into one before the actual merge happens? |
No, it's fine. Each commit actually follows reasonable work units. |
estebank
requested changes
May 28, 2019
|
Does this PR change how code is broken up into tokens? (It shouldn't.) |
Do you have a prefered folder for that testcase (and name maybe)? |
|
Unfortunately it does :( I'll try to fix that tomorrow. |
estebank
reviewed
May 29, 2019
| @@ -2,9 +2,7 @@ error: unterminated raw string | |||
| --> $DIR/raw-str-unterminated.rs:2:5 | |||
| | | |||
| LL | r#" string literal goes on | |||
| | ^ unterminated raw string | |||
| | | |||
| = note: this raw string should be terminated with `"#` | |||
There was a problem hiding this comment.
Maybe we should keep the current note for this case where no likely close is found.
|
@petrochenkov good catch. Wasn't aware of that behavior.
The parser now tracks what parser is doing the parsing, and @petrochenkov will probably be able to answer this with certainty, but I believe that you might be able check |
Sadly that's not possible, if I'm correct. rust/src/libsyntax/parse/parser.rs Lines 196 to 236 in c28084a but I'm not using the |
|
Don't consume the extra |
|
☔ The latest upstream changes (presumably #61418) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Sorry, I was away over the last couple of days (holidays in germany :) ). I'm looking into this probably wendsday. I'll going to rebase everything. @estebank Should I remove the error completly or should I instead print a warning? Should I just comment out the code, but leave it there, or remove it? |
|
If possible, I'd love for these changes to also account for #60793, which unifies the (raw/raw byte) actual scanning and defers the error reporting/validation step. It'd be great to keep these separate, if possible ❤️ |
Leave the prior error behavior there.
Remove the code, there shouldn't ever be commented out code, it only raises questions like "why is this here" or "why wasn't this deleted". Follow the path of least resistance. It's preferable to have solve a part of the whole problem and the rest later than letting the whole problem linger on. |
Dylan-DPC-zz
added
S-waiting-on-author
|
ping from triage @hellow554 any updates on this? |
|
@Dylan-DPC not really. Problem is that I hadn't time in a while and last week I started looking at this again. #60793 changed a lot. Need some time to look at the code again. Will update this. |
hellow554
force-pushed
the
raw_string_diag
branch
from
e63f19d
to
d94b2ff
Compare
|
Hey @estebank , long time no seen 😊 |
|
cc @matklad wrt changed raw strings lexer code |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
hellow554
force-pushed
the
raw_string_diag
branch
from
d94b2ff
to
89bab08
Compare
hellow554
force-pushed
the
raw_string_diag
branch
from
89bab08
to
0719d0d
Compare
|
@estebank do you have some time to spare? |
estebank
reviewed
Jul 26, 2019
| help: you might have meant to end the raw string here | ||
| | | ||
| LL | '##; | ||
| | ^^^ |
There was a problem hiding this comment.
I'm worried about the '## not being "##.
There was a problem hiding this comment.
True, the reason for it is, that I look for #, not for "#[...]. I think I can change this.
estebank
reviewed
Jul 26, 2019
|
|
||
| fn main() { | ||
| m!(r#"abc"##); | ||
| } |
There was a problem hiding this comment.
Can we add another tests with the following combinations?
macro_rules! m {
($tt:tt) => ()
}
fn main() {
m!(r#"abc"##);
}
macro_rules! m {
($tt:tt #) => ()
}
fn main() {
m!(r#"abc"#);
}
Just want us to catch any regressions in the future.
What can I help with? |
Reviewing this PR ^^ Sorry, if I used the not-quite-correct terms :) |
estebank
reviewed
Jul 29, 2019
src/libsyntax/parse/lexer/mod.rs
Outdated
| fn fail_unterminated_raw_string(&self, start: Span, hash_count: u16, spans: Vec<Span>) -> ! { | ||
| const SPAN_THRESHOLD: usize = 3; | ||
| const MSG_STR: &str = "you might have meant to end the raw string here"; | ||
| let hash_str = format!("\'{}", "#".repeat(hash_count as usize)); |
There was a problem hiding this comment.
Suggested change
| let hash_str = format!("\'{}", "#".repeat(hash_count as usize)); | |
| let hash_str = format!("\"{}", "#".repeat(hash_count as usize)); |
The lexer now emits a suggestion for `cargo fix`
hellow554
force-pushed
the
raw_string_diag
branch
from
0719d0d
to
0c00d9f
Compare
|
With #59706 a lot has changed. I'm not sure if I can keep up with it. :/ |
|
Ping from triage: Hi @hellow554, Have you had a chance to look at #59706? |
|
@Alexendoo unfortunately no, I'm going to close this, because I don't feel like I have the time for this. Sorry to say. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR tries to improve unbalanced raw string literals, e.g. too many or too little hashtags in the end.
Unified parsing of rwa byte and raw string literals by introducing a new enum. The difference is that '\r' cannot appear solely in raw strings and that raw byte is ascii only.
Resolves #60762
r? @estebank