Mark binding-pattern identifiers with their declaration kind so local
destructuring assignments use the normal TDZ and const assignment path.
This makes local const destructuring match the environment-backed path.
Also teach GetById to expose primitive string virtual index properties
before boxing, matching StringGetOwnProperty and GetByValue. Together
these fix the SpiderMonkey for-in/of const declaration coverage and the
lexical destructuring TDZ test.
Treat ordinary and generator function expressions inside class static
blocks as boundaries for await binding names, while still rejecting
await as an arrow parameter in the static block itself.
Parse field initializers without inheriting an enclosing async
function's await-expression context, so script field initializers can
resolve await as an identifier while computed field names still use the
enclosing expression context.
Cover static block function expressions, class field initializers, and
the AST shape for an await arrow inside a nested function.
The parser only set `might_need_arguments_object` when an `arguments`
or `eval` Identifier went through `consume()`, but shorthand object
properties create the reference via `make_identifier()` directly. As
a result `function f() { return { arguments } }` allocated an
`arguments` local, never initialized it, and crashed at runtime when
the property was read.
Fall back to scope-driven detection: if scope analysis allocated a
non-lexical `arguments` local for the function, treat it as a real
arguments-object reference and emit `CreateArguments`. Skip the
fallback when a function declaration named `arguments` claims the
local, since that local belongs to the function, not the arguments
object.
Add a runtime test covering shorthand inside a free function and a
method, plus a regression test for `({ eval } = ...)` to confirm
destructuring assignment doesn't accidentally trigger arguments
materialization.
The scope collector stored identifier_groups and variables in
HashMaps and then sorted them alphabetically before assigning local
register indices. The sorts existed only because HashMap iteration
order is non-deterministic; alphabetical was a stable choice for
comparing bytecode against the now-removed C++ port.
Switch both maps to indexmap::IndexMap so iteration follows the order
of first reference (= source order), and drop the alphabetical sorts.
Local indices now reflect declaration order, which matches what shows
up in bytecode dumps and is easier to read alongside the source.
Add a focused bytecode test using zebra/yak/aardvark to pin the new
allocation order; existing tests using let/var declarations have
their local indices renumbered to match.
The Rust parser used to copy several "rule_start"-derived positions
from the C++ implementation: every identifier inside a binding pattern
inherited the pattern's `[`/`{` position, every property identifier
after `.` inherited the period's position, every spread element
inherited the surrounding `[`/`{` position, and identifier-name
property keys inherited the object/class start position. This was
useful while comparing bytecode against the C++ port; with the C++
side gone, those quirks just hide the actual source positions in
source maps and devtools.
Drop the dedicated `binding_pattern_start` parser field and the
`ident_pos_override` parameter on `parse_property_key`, and capture
each identifier's own start position at the consume site.
Add an AST snapshot test that pins the new per-identifier positions
for object, array, nested, and parameter binding patterns.
The parser used to suppress the arguments/eval reference check via a
state flag that was set during the entire `parse_property_key` call.
That was over-broad: identifiers inside a computed property key like
`{ [arguments]: 1 }` are real references, but the flag silenced their
check too, leaving the function unmarked as needing the arguments
object. Reading the resulting property at runtime crashed.
Replace the flag with a `consume_property_key_token()` method used at
the specific consume sites for the property key token itself, so the
suppression is narrow. Inner consumes inside computed keys now go
through regular `consume()` and run the check normally.
Add a focused AST snapshot test covering plain, shorthand, computed,
binding-pattern, and method-name property-key cases.
Carry full source positions through the Rust bytecode source map so
stack traces and other bytecode-backed source lookups can use them
directly.
This keeps exception-heavy paths from reconstructing line and column
information through SourceCode::range_from_offsets(), which can spend a
lot of time building SourceCode's position cache on first use.
We're trading some space for time here, but I believe it's worth it at
this tag, as this saves ~250ms of main thread time while loading
https://x.com/ on my Linux machine. :^)
Reading the stored Position out of the source map directly also exposed
two things masked by the old range_from_offsets() path: a latent
off-by-one in Lexer::new_at_offset() (its consume() bumped line_column
past the character at offset; only synthesize_binding_pattern() hit it),
and a (1,1) fallback in range_from_offsets() that fired whenever the
queried range reached EOF. Fix the lexer, then rebaseline both the
bytecode dump tests (no more spurious "1:1") and the destructuring AST
tests (binding-pattern identifiers now report their real columns).
Cache failed arrow function attempts by token offset. Once we
determine that '(' at offset N is not the start of an arrow
function, skip re-attempting at the same offset.
Without memoization, nested expressions like (a=(b=(c=(d=0))))
cause exponential work: each failed arrow attempt at an outer '('
re-parses all inner '(' positions during grouping expression
re-parse, and each inner position triggers its own arrow
attempts. With n nesting levels, the innermost position is
processed O(2^n) times.
The C++ parser already has this optimization (via the
try_parse_arrow_function_expression_failed_at_position()
memoization cache).
When the parser speculatively tries to parse an arrow function
expression, encountering `this` inside a default parameter value
like `(a = this)` propagates uses_this flags to ancestor function
scopes via set_uses_this(). If the arrow attempt fails (no =>
follows), these flags were left behind, incorrectly marking
ancestor scopes as using `this`.
Fix this by saving and restoring the uses_this and
uses_this_from_environment flags on all ancestor function scopes
around speculative arrow function parsing.
The parser was not calling add_declaration() for using declarations in
for-loop initializers, causing the scope collector to miss them. This
meant identifiers declared with `using` in for-loops were incorrectly
resolved as globals instead of local variables.
When parsing object expressions like {x: 42}, the parser was eagerly
registering the identifier "x" in the scope collector even though it's
only used as a property key and not a variable reference.
Fix this by deferring the registration until we know we're dealing with
a shorthand property ({x}), where the identifier is actually a variable
reference that needs to be captured by the scope collector.
The scope collector uses HashMaps for identifier groups and variables,
which means their iteration order is non-deterministic. This causes
local variable indices and function declaration instantiation (FDI)
bytecode to vary between runs.
Fix this by sorting identifier group keys alphabetically before
assigning local variable indices, and sorting vars_to_initialize by
name before emitting FDI bytecode.
Also make register allocation deterministic by always picking the
lowest-numbered free register instead of whichever one happens to be
at the end of the free list.
This is preparation for bringing in a new source->bytecode pipeline
written in Rust. Checking for regressions is significantly easier
if we can expect identical output from both pipelines.
Rework our hash functions a bit for significant better performance:
* Rename int_hash to u32_hash to mirror u64_hash.
* Make pair_int_hash call u64_hash instead of multiple u32_hash()es.
* Implement MurmurHash3's fmix32 and fmix64 for u32_hash and u64_hash.
On my machine, this speeds up u32_hash by 20%, u64_hash by ~290%, and
pair_int_hash by ~260%.
We lose the property that an input of 0 results in something that is not
0. I've experimented with an offset to both hash functions, but it
resulted in a measurable performance degradation for u64_hash. If
there's a good use case for 0 not to result in 0, we can always add in
that offset as a countermeasure in the future.
Both return paths from parse_import_statement() were missing a call to
consume_or_insert_semicolon(), causing explicit semicolons to be left
unconsumed and parsed as spurious EmptyStatements.
Import statements fail to consume their trailing semicolons, resulting
in spurious EmptyStatement nodes in the AST. This will be fixed in the
next commit.
The TaggedTemplateLiteral node incorrectly gets its source range from
the class declaration start rather than the extends expression start.
This will be fixed in the next commit.
When a function has parameter expressions (default values), body var
declarations that shadow a name referenced in a default parameter
expression must not be optimized to local variables. The default
expression needs to resolve the name from the outer scope via the
environment chain, not read the uninitialized local.
We now mark identifiers referenced during formal parameter parsing
with an IsReferencedInFormalParameters flag, and skip local variable
optimization for body vars that carry both this flag and IsVar (but
not IsForbiddenLexical, which indicates parameter names themselves).
When hex, octal, or binary integer literals overflow u64, we used to
fall back to UINT64_MAX. This produced incorrect results for any value
larger than 2^64.
Fix this by accumulating the value as a double digit-by-digit when
the u64 parse fails.
When parsing class elements, after consuming the `static` keyword, the
parser would unconditionally consume `async` if it appeared next. This
meant that for `static async()`, where `async` is the method name (not
a modifier), the `async` token was consumed too early and its source
position was lost.
Fix this by applying the same lookahead check used for top-level async
detection: only consume `async` as a modifier if the following token is
not `(`, `;`, `}`, or preceded by a line terminator. This lets `async`
be parsed as a property key with its correct source position.
When an identifier was registered and its group already existed but
had no declaration_kind set, we failed to propagate it. This caused
var declarations to lose their annotation in AST dumps when the
identifier was referenced before its declaration.
When a function accesses the arguments object in non-strict mode, scope
analysis was skipping argument index assignment for all parameter
candidates. This is correct for regular parameters (which participate in
the sloppy-mode arguments-parameter linkage), but rest parameters never
participate in that linkage and should always get their argument index.
Test that destructured parameter bindings correctly do NOT receive
local indices when defeated by closure capture, direct eval, with
statements, or arguments object access.
Destructured parameter bindings (e.g. the x and y in function f([x, y]))
were not receiving any local indices during scope analysis. This meant
they could not benefit from the fast local variable access path in the
bytecode interpreter.
The issue had two parts:
1. set_function_parameters() only registered plain Identifier parameters
with the IsParameterCandidate flag. BindingPattern parameters only
got IsForbiddenLexical, which caused their identifiers to be skipped
entirely during resolve_identifiers().
2. resolve_identifiers() unconditionally called set_argument_index() for
all parameter candidates, but get_index_of_parameter_name() only
finds plain Identifier parameters, not bindings inside patterns.
Fix this by registering destructured binding identifiers with
IsParameterCandidate so they participate in resolution, and then falling
back to a local variable slot when get_index_of_parameter_name() does
not find them. This is correct because the argument register holds the
whole object/array to be destructured, while the individual bindings
live in local variable slots.
Add both runtime (test-js) and AST dump (test-js-ast) coverage for
destructured parameter bindings. The runtime tests verify that functions
with destructured parameters produce correct results. The AST tests
show that destructured parameter bindings currently receive no local
index annotations, unlike plain parameters which get [argument:N].
has_declaration_in_current_function() only checked IsLexical and
IsVar flags, but parameters carry IsParameterCandidate instead.
This meant a parameter named "arguments" wasn't recognized as a
declaration, causing the scope analysis to incorrectly treat it
as an access to the arguments exotic object instead of a plain
parameter binding.
Add IsParameterCandidate to the flags check so parameters are
correctly recognized, allowing "arguments" to get its [argument:N]
index like any other parameter.
Add a test case showing that a parameter named "arguments" as the
second parameter should get [argument:1], demonstrating the current
bug where parameter-named "arguments" identifiers lose their
argument index annotations.
Replace the ScopePusher RAII class (which performed scope analysis
in its destructor chain during parsing) with a two-phase approach:
1. ScopeCollector builds a tree of ScopeRecord nodes during parsing
via RAII ScopeHandle objects. It records declarations, identifier
references, and flags, but does not resolve anything.
2. After parsing completes, ScopeCollector::analyze() walks the tree
bottom-up and performs all resolution: propagate eval/with
poisoning, resolve identifiers to locals/globals/arguments, hoist
functions (Annex B.3.3), and build FunctionScopeData.
Key design decisions:
- ScopeRecord::ast_node is a RefPtr<ScopeNode> to prevent
use-after-free when synthesize_binding_pattern re-parses an
expression as a binding pattern (the original parse's scope records
survive with stale AST node pointers).
- Parser::scope_collector() returns the override collector if set
(for synthesize_binding_pattern's nested parser), ensuring all
scope operations route to the outer parser's scope tree.
- FunctionNode::local_variables_names() delegates to its body's
ScopeNode rather than copying at parse time, since analysis runs
after parsing.
Add a test runner for AST dump tests, modeled after test-js-bytecode.
Tests consist of JS input files and expected AST dump output, verified
by parsing with --parse-only --dump-ast --disable-ansi-colors.
