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

RawString error reporting cleanup #72884

Merged
merged 2 commits into from
Jun 2, 2020

Conversation

Julian-Wollersberger
Copy link
Contributor

@Julian-Wollersberger Julian-Wollersberger commented Jun 1, 2020

I simplified how errors with raw string are represented in the lexer and reportet in the parser, by using one enum instead of two structs with impls. This makes 70 code lines obsolete.

I also noticed some other things (2nd commit) and added a missing test for the `too many '#' symbols' error.

My original intent was to improve performance, but the only thing I found was to inline some functions in cursor.rs. It's effect is barely measurable, though.

There is one open question. Before, the compiler aborts when encountering the too many '#' symbols error. Now the lexer says in this case that there are 0 hashes, and then later the parser aborts on the error.
I'm worrying that the parser may be changed to recover and continue, and then later stages will see the wrong numer of hashes and act strange. (eg. the format! macro expansion).
Is that possibility important enough today to worry about it?

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 1, 2020
@petrochenkov
Copy link
Contributor

r? @petrochenkov
cc @matklad

@rust-highfive

This comment has been minimized.

src/librustc_lexer/src/lib.rs Outdated Show resolved Hide resolved
src/test/ui/parser/raw/raw-str-too-many-hashes.rs Outdated Show resolved Hide resolved
src/librustc_parse/lexer/mod.rs Outdated Show resolved Hide resolved
src/librustc_lexer/src/cursor.rs Outdated Show resolved Hide resolved
src/librustc_lexer/src/cursor.rs Outdated Show resolved Hide resolved
src/librustc_lexer/src/cursor.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 1, 2020
@Julian-Wollersberger Julian-Wollersberger force-pushed the raw_str_error_cleanup branch 2 times, most recently from 5309fbe to 725d2cd Compare June 1, 2020 19:54
This makes `UnvalidatedRawStr` and `ValidatedRawStr` unnecessary and removes 70 lines.
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 2, 2020

📌 Commit 7be8077 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 2, 2020
@Julian-Wollersberger
Copy link
Contributor Author

Thanks for reviewing ^^

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2020
Rollup of 4 pull requests

Successful merges:

 - rust-lang#72884 (RawString error reporting cleanup )
 - rust-lang#72888 (Add a warning about infinite reading in read_(until|line))
 - rust-lang#72914 (Minor: off-by-one error in RELEASES.md)
 - rust-lang#72916 (Update README.md)

Failed merges:

r? @ghost
@bors bors merged commit 466d3e7 into rust-lang:master Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants