-
-
Notifications
You must be signed in to change notification settings - Fork 537
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
fix(lexer): incorrect lexing of large hex/octal/binary literals #4072
Conversation
Your org has enabled the Graphite merge queue for merging into mainAdd the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
CodSpeed Performance ReportMerging #4072 will not alter performanceComparing Summary
|
Hi @DonIsaac. Thanks very much for tackling this. Apologies if some of my comments above appear nit-picky. Maybe they are, given that these functions are not in a hot path, so a few extra instructions aren't a bit deal. But I just figured that since I noticed these things, I might as well point them out. Feel free to ignore them if you don't feel it's worth the bother. One other thing: Could you please add a comment at the top somewhere explaining what the reason for the fast path/slow path setup is, so a future reader can follow why it's like this? |
@DonIsaac Can you wrap up this PR? |
Nice one @DonIsaac. If CI passes, and you have no further changes to make, feel free to merge this. I don't think any need to wait for Boshen to review. |
Merge activity
|
Closes #3347. Implementation follows the approach described by @overlookmotel [here](#3347 (comment)).
e10a6ca
to
4a656c3
Compare
## [0.20.0] - 2024-07-11 - 5731e39 ast: [**BREAKING**] Store span details inside comment struct (#4132) (Luca Bruno) ### Features - 67fe75e ast, ast_codegen: Pass the `scope_id` to the `enter_scope` event. (#4168) (rzvxa) - 54cd04a minifier: Implement dce with var hoisting (#4160) (Boshen) - 44a894a minifier: Implement return statement dce (#4155) (Boshen) - 725571a napi/transformer: Add `jsx` option to force parsing with jsx (#4133) (Boshen) ### Bug Fixes - 48947a2 ast: Put `decorators` before everything else. (#4143) (rzvxa) - 7a059ab cfg: Double resolution of labeled statements. (#4177) (rzvxa) - 4a656c3 lexer: Incorrect lexing of large hex/octal/binary literals (#4072) (DonIsaac) - 28eeee0 parser: Fix asi error diagnostic pointing at invalid text causing crash (#4163) (Boshen) ### Performance - ddfa343 diagnostic: Use `Cow<'static, str>` over `String` (#4175) (DonIsaac) - 2203143 semantic: Store unresolved refs in a stack (#4162) (lucab) - fca9706 semantic: Faster search for leading comments (#4140) (Boshen) ### Documentation - bdcc298 ast: Update the note regarding the `ast_codegen` markers. (#4149) (rzvxa) ### Refactor - 03ad1e3 semantic: Tweak comment argument type (#4157) (lucab) Co-authored-by: Boshen <Boshen@users.noreply.github.com>
Closes #3347. Implementation follows the approach described by @overlookmotel here.