fix(ast_tools): support u128 in assert_layouts generator#17050
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
8499052 to
919d133
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for u128 and i128 types in the AST generators. Previously, these 128-bit integer types caused panics because their alignment varied across Rust versions. Now that all supported Rust versions have consistent alignment of 16 for these types, they can be safely supported.
Key Changes:
- Removed panic statements for u128/i128 types in the
assert_layoutsgenerator and replaced them with proper layout calculations - Added u128 deserialization logic to both
raw_transferandraw_transfer_lazygenerators - Updated generated JavaScript with improved formatting comments
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tasks/ast_tools/src/generators/assert_layouts.rs | Replaced panic statements with proper Layout calculations for u128, i128, NonZeroU128, and NonZeroI128 types |
| tasks/ast_tools/src/generators/raw_transfer.rs | Added u128 deserialization function and improved u64 formatting with clarifying comments |
| tasks/ast_tools/src/generators/raw_transfer_lazy.rs | Added u128 deserialization function and improved u64 formatting with clarifying comments |
| napi/parser/src-js/generated/lazy/constructors.js | Updated generated u64 constructor with clarifying comment for the 2^32 constant |
Critical Issue Found: The constant used for 2^64 in the u128 deserialization logic is incorrect in both raw_transfer.rs and raw_transfer_lazy.rs. The value 18446744073709552000 should be 18446744073709551616 (a difference of 384), which will cause incorrect deserialization of u128 values.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
919d133 to
98dcb91
Compare
Merge activity
|
Previously `u128` was not supported in `assert_layouts` generator, because Rust changed the alignment of `u128` from 8 to 16 - and therefore the alignment depended on Rust version. That change happened some time ago now, and all Rust versions above our MSRV have `u128` with alignment of 16. So we can now support `u128` (and also `i128`, `NonZeroU128`, `NonZeroI128`). Also support `u128` in `raw_transfer` and `raw_transfer_lazy` generators. This will help with making lexer tokens serializable (#17025), since `Token` contains a `u128`.
98dcb91 to
a92faf0
Compare
### 🚀 Features - d209c21 allocator: Add cap to FixedSizeAllocatorPool and block when exhausted (#17023) (Cameron) - fb2af91 allocator: Add bitset utils (#17042) (zhaoting zhou) - c16082c tasks/compat_data: Integrate `node-compat-table` (#16831) (Boshen) - 5586823 span: Extract TS declaration file check to its own function (#17037) (camchenry) - 3d2b492 minifier: Fold iife arrow functions in call expressions (#16477) (Armano) - 67e9f9e codegen: Keep comments on the export specifiers (#16943) (夕舞八弦) - cb515fa parser: Improve error message for `yield` as identifier usage (#16950) (sapphi-red) - dcc856b parser: Add help for `new_dynamic_import` error (#16949) (sapphi-red) - c3c79f8 parser: Improve import attribute value error message (#16948) (sapphi-red) - 291b57b ast_tools: Generate TS declaration files for deserializer and walk files (#16912) (camc314) - 74eae13 minifier: Remove unused import specifiers (#16797) (camc314) ### 🐛 Bug Fixes - fb9e193 linter: OOM problems with custom plugins (#17082) (overlookmotel) - e59132b parser/napi: Fix lazy deser (#17069) (overlookmotel) - a92faf0 ast_tools: Support `u128` in `assert_layouts` generator (#17050) (overlookmotel) - 47b4c2f minifier/docs: Correct hyperlink path in OPTIMIZATIONS.md (#16986) (GRK) - 3002649 transformer/typescript: Remove unused import equals declaration (#16776) (Dunqing) - 5a2af88 regular_expression: Correct named capture group reference error (#16952) (sapphi-red) ### ⚡ Performance - b657bb6 allocator: Reduce time `Mutex` lock is held in `FixedSizeAllocatorPool::get` (#17079) (overlookmotel) - 1f3b19b ast: `#[ast]` macro use `#[repr(transparent)]` for single-field structs (#17052) (overlookmotel) - 225f229 parser: Use SmallVec for duplicate default export detection (#16801) (camc314) ### 📚 Documentation - a9c419f traverse: Update safety comments (#16944) (overlookmotel) Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>

Previously
u128was not supported inassert_layoutsgenerator, because Rust changed the alignment ofu128from 8 to 16 - and therefore the alignment depended on Rust version.That change happened some time ago now, and all Rust versions above our MSRV have
u128with alignment of 16. So we can now supportu128(and alsoi128,NonZeroU128,NonZeroI128).Also support
u128inraw_transferandraw_transfer_lazygenerators.This will help with making lexer tokens serializable (#17025), since
Tokencontains au128.