chore(parser): increase size of TokenSet #7997
Conversation
|
WalkthroughTokenSet storage expanded from two 128-bit lanes to three, increasing capacity. All constructors, operations, and the EMPTY constant updated to support the new three-lane representation. Union operations now combine three lanes instead of two. A new public Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_parser/src/token_set.rs (1)
25-34: Add documentation for the public method.The implementation is correct, but this new public method lacks a doc comment explaining its purpose and behaviour.
Apply this diff to add documentation:
+ /// Returns `true` if the set contains the specified token kind. + /// + /// # Examples + /// + /// ``` + /// # use biome_parser::TokenSet; + /// # use biome_rowan::SyntaxKind; + /// // let set = token_set![Kind::Foo, Kind::Bar]; + /// // assert!(set.contains(Kind::Foo)); + /// ``` pub fn contains(&self, kind: K) -> bool {Optional: Consider making this
const fnfor consistency withunionand to enable use in const contexts, though the current implementation is perfectly adequate.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_parser/src/token_set.rs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: autofix
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
🔇 Additional comments (4)
crates/biome_parser/src/token_set.rs (4)
5-8: Struct expansion looks sound.The three-lane storage correctly increases capacity to 384 token kinds, and the EMPTY constant properly initialises all lanes to zero.
14-23: Union operation correctly extended.All three lanes are properly combined with bitwise OR.
48-56: Mask function correctly distributes across three lanes.The bit shifting and offset calculations are accurate for each range (0..=127, 128..=255, 256..=383), with appropriate panic behaviour for invalid input.
68-235: Excellent test coverage!The test suite thoroughly validates all three lanes with boundary tests, membership checks, union operations, and panic behaviour. Well done on the comprehensive testing—this gives confidence that the three-lane expansion works correctly.
CodSpeed Performance ReportMerging #7997 will not alter performanceComparing Summary
Footnotes
|
|
Will fix the lint issue later today when I have time |
2 participants
Summary
This PR is related to #7994
Similar to the other listed PR we need to increase the size of the TokenKind limit to handling growing language grammars. This issue was caught while working on the CSS
iffeature for issue #6725 in unit tests such asok::grit_metavariable::metavar_cssincrates/biome_parser/src/token_set.rsdue to an "attempt to shift left with overflow" panic. This PR adds an extra item to the TokenKind array increasing the limit from 256 kinds to 384. I also broke this work out into a smaller separate PR since it seems like a deeper change with potentially more widespread impact and wanted it to be easier to review and scrutinize.Test Plan
Docs
n/a
AI Assistance Disclosure
Claude Code was used for basic iteration and code research but almost all of this code was written directly by me and verified with unit tests written by me as well.