-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Make some fatal lexer errors recoverable #33199
Conversation
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
@@ -27,26 +27,47 @@ pub use ext::tt::transcribe::{TtReader, new_tt_reader, new_tt_reader_with_doc_fl | |||
pub mod comments; | |||
mod unicode_chars; | |||
|
|||
pub type TokenAndSpanResult = Result<TokenAndSpan, ()>; |
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.
This doesn't seem worth making a type for, it only saves a few chars.
There are some small things to change inline, r+ with those fixed. |
ed6c1f9
to
d420225
Compare
I tried using a closure, but that didn't really work out. Passing just the result to I've also added a note to the rustdoc warning. |
6cf22db
to
38b45dd
Compare
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
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.
Could you add a comment to this test explaining what is being tested please?
@mitaa thanks for the changes and explanation, I have one more very minor request (sorry I missed it the first time round), then r+ for reals. |
38b45dd
to
6887202
Compare
(updated) |
@bors: r+ |
📌 Commit 6887202 has been approved by |
I've kept the changes to a minimum since I'm not really sure if this approach is a acceptable.
fixes #12834
cc @nrc