A few cleanups and minor improvements for the lexer #53289
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
There was a problem hiding this comment.
Thanks for the PR, @ljedrz!
I left some comments where I think that things should stay as they are.
| fn read_block_comment(rdr: &mut StringReader, | ||
| code_to_the_left: bool, | ||
| comments: &mut Vec<Comment>) { | ||
| fn read_block_comment(rdr: &mut StringReader, code_to_the_left: bool, comments: &mut Vec<Comment>) { |
There was a problem hiding this comment.
I think the previous version is actually more readable.
src/libsyntax/parse/lexer/mod.rs
Outdated
| } | ||
|
|
||
| assert!(self.fatal_errs.is_empty()); |
There was a problem hiding this comment.
I'd rather keep the assertion in try_next_token() since it's also called from other places.
src/libsyntax/parse/lexer/mod.rs
Outdated
| } | ||
| _ => break, | ||
| } | ||
| while t.tok.is_irrelevant() { |
There was a problem hiding this comment.
I think, I'd rather keep the previous version. It's not immediately clear what is_irrelevant() means and the token might be "relevant" in other contexts, so pulling it into a method of Token could be confusing.
|
Thanks, comments addressed. |
|
@michaelwoerister are you sure about that |
|
I'd rather stay on the safe side here. An assertion like this is practically free as far as runtime performance goes. |
|
@michaelwoerister ah, that's true; that check is trivial. |
|
@bors r+ |
|
📌 Commit 40d9bd0 has been approved by |
bors
added
S-waiting-on-bors
…ister A few cleanups and minor improvements for the lexer - improve readability by adjusting the formatting of some function signatures and adding some newlines - reorder some functions for easier reading - remove redundant `'static` in `const`s - remove some explicit `return`s - read directly to a `String` in `gather_comments_and_literals` - change `unwrap_or!` (macro) to `unwrap_or` (function) - move an `assert!`ion from `try_next_token` (called in a loop) to `try_real_token` after all calls to `try_next_token` - `#[inline]` some one-liner functions - assign directly from an `if-else` expression - refactor a `match` to `map_or` - add a `token::is_irrelevant` function to detect tokens that are not "`real`"
A few cleanups and minor improvements for the lexer - improve readability by adjusting the formatting of some function signatures and adding some newlines - reorder some functions for easier reading - remove redundant `'static` in `const`s - remove some explicit `return`s - read directly to a `String` in `gather_comments_and_literals` - change `unwrap_or!` (macro) to `unwrap_or` (function) - move an `assert!`ion from `try_next_token` (called in a loop) to `try_real_token` after all calls to `try_next_token` - `#[inline]` some one-liner functions - assign directly from an `if-else` expression - refactor a `match` to `map_or` - add a `token::is_irrelevant` function to detect tokens that are not "`real`"
|
☀️ Test successful - status-appveyor, status-travis |
'staticinconstsreturnsStringingather_comments_and_literalsunwrap_or!(macro) tounwrap_or(function)assert!ion fromtry_next_token(called in a loop) totry_real_tokenafter all calls totry_next_token#[inline]some one-liner functionsif-elseexpressionmatchtomap_ortoken::is_irrelevantfunction to detect tokens that are not "real"