-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
refactor(parser): implement NthToken for T #2727
Conversation
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
CodSpeed Performance ReportMerging #2727 will not alter performanceComparing Summary
|
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.
Oh, you managed to kill the lifetime! (To reduce it during lexing)
I did try to do that some time ago but then stopped.
Great refactor! Is there some documentation (contribution guide) we need to update?
crates/biome_parser/src/lexer.rs
Outdated
/// Creates a new [BufferedLexer] wrapping the passed in [Lexer]. | ||
pub fn new(lexer: Lex) -> Self { | ||
Self { | ||
inner: lexer, | ||
current: None, | ||
lookahead: VecDeque::new(), | ||
lookahead: Lookahead::with_capacity(5), |
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.
Why 5?
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.
Oh,
I wanted to look at the performance tests, but it seems there is no effect,
Do you think it's better to use just new
?
5 is a the longest lookahed
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.
Yeah, if there's no change in performance, maybe we should stick to new
Summary
We have code duplication around the non-trivia lookahead in our parser. The MR attempts to extract the common parts into the parser crate. Basically, we have a
non_trivia_lookahead
vector for each token source. We can try to move them inside the current lookahead in the buffered lexer, because we are interested only in non-trivia lookahead.Test Plan
cargo test