This error was found by asking an LLM to generate additional, related
test cases for the bug affecting https://volkswagen.de fixed in an
earlier commit.
An unconditional call to `copy_if_needed_to_preserve_evaluation_order`
in this place was showing up quiet significantly in the JS benchmarks.
To avoid the regression, there is now a small heuristic that avoids the
unnecessary Mov instruction in the vast majority of cases. This is
likely not the best way to deal with this. But the changes in the
current patch set are focussed on correctness, not performance. So I
opted for a localized, minimal-impact solution to the performance
regression.
This error was found by asking an LLM to generate additional, related
test cases for the bug affecting https://volkswagen.de fixed in an
earlier commit.
This error was found by asking an LLM to generate additional, related
test cases for the bug affecting https://volkswagen.de fixed in an
earlier commit.
Replace 20 separate Put instructions (5 PutKinds x 4 forms) with
4 unified instructions (PutById, PutByIdWithThis, PutByValue,
PutByValueWithThis), each carrying a PutKind field at runtime instead
of being a separate opcode.
This reduces the number of handler entry points in the dispatch loop
and eliminates template instantiations of put_by_property_key and
put_by_value that were being duplicated 5x each when inlined by LTO.
For computed class fields, field_name is empty and the name is set at
runtime. Avoid setting pending_lhs_name in that case, which prevents
the name from leaking into computed field initializers.
When the left-hand side of an assignment, update, or for-in loop is
invalid (e.g. `foo() = "bar"`), the bytecode generator emits a Throw
instruction. Previously, it would also create a dead basic block after
the Throw, resulting in unreachable instructions in the output.
Fix this by returning early from the relevant codegen paths after
emitting the Throw, and by guarding for-in/for-of body generation
with an is_current_block_terminated() check.
For `export default (class Name { })`, two things were wrong:
The parser extracted the class expression's name as the export's
local binding name instead of `*default*`. Per the spec, this is
`export default AssignmentExpression ;` whose BoundNames is
`*default*`, not the class name.
The bytecode generator had a special case for ClassExpression that
skipped emitting InitializeLexicalBinding for named classes.
These two bugs compensated for each other (no crash, but wrong
behavior). Fix both: always use `*default*` as the local binding
name for expression exports, and always emit InitializeLexicalBinding
for the `*default*` binding.
When a statement in a switch case body doesn't produce a result (e.g.
a variable declaration), we were incorrectly resetting the completion
value to undefined. This caused the completion value of preceding
expression statements to be lost.
Move the duplicated ThrowIfTDZ emission logic from three places in
ASTCodegen.cpp into a single Generator::emit_tdz_check_if_needed()
helper. This handles both argument TDZ (which requires a Mov to
empty first) and lexically-declared variable TDZ uniformly.
This avoids emitting some unnecessary ThrowIfTDZ instructions.
When MemberExpression::generate_bytecode calls emit_load_from_reference,
it only uses the loaded_value and discards the reference operands. For
computed member expressions (e.g. a[0]), this was generating an
unnecessary Mov to save the property register for potential store-back.
Add a ReferenceMode parameter to emit_load_from_reference. When LoadOnly
is passed, the computed property path skips the register save and Mov.
Per AssignmentRestElement and AssignmentElement in the specification,
the DestructuringAssignmentTarget reference must be evaluated before
iterating or stepping the iterator. We were doing it in the wrong
order, which caused observable differences when the target evaluation
has side effects, and could lead to infinite loops when the iterator
never completes.
Add Generator::emit_evaluate_reference() to evaluate a member
expression's base and property into ReferenceOperands without performing
a load or store, then use the pre-evaluated reference for the store
after iteration completes.
The AsyncIteratorClose bytecode op calls async_iterator_close() which
uses synchronous await() internally. This spins the event loop while
execution contexts are on the stack, violating the microtask checkpoint
assertion in LibWeb.
Replace AsyncIteratorClose op emissions in for-await-of close handlers
with inline bytecode that uses the proper Await op, allowing the async
function to yield and resume naturally through the event loop.
For the non-throw path (break/return/continue-to-outer): emit
GetMethod, Call, Await, and ThrowIfNotObject inline.
For the throw path: wrap the close steps in an exception handler so
that any error from GetMethod/Call/Await is discarded and the original
exception is rethrown, per spec step 5.
The else branch already throws ReferenceError and switches to a dead
basic block, so the emit_todo() in the PutValue section is unreachable.
Return early after the throw and replace emit_todo() with
VERIFY_NOT_REACHED().
CallExpression is accepted as an assignment target for web compatibility
(Annex B), but must throw ReferenceError at runtime. We were incorrectly
throwing TypeError with a TODO message.
Replace emit_todo() calls in three codegen paths (simple assignment,
compound assignment/update, and for-in/of) with proper ReferenceError
using the "Invalid left-hand side in assignment" message, matching the
behavior of V8 and JSC.
When a for-of or for-await-of loop exits via break, return, throw,
or continue-to-outer-loop, we now correctly call IteratorClose
(or AsyncIteratorClose) to give the iterator a chance to clean
up resources.
This uses a synthetic FinallyContext that wraps the LHS assignment
and loop body, reusing the existing try/finally completion record
machinery. The ReturnToFinally boundary is placed between Break
and Continue so that continue-to-same-loop bypasses the close
(zero overhead on normal iteration) while all other abrupt exits
route through the iterator close dispatch chain.
for-in (enumerate) does not require iterator close per spec.
Change the completion_value field from Optional<Value> to Operand
in both IteratorClose and AsyncIteratorClose bytecode instructions.
This allows passing a dynamic value from a register, which is needed
for iterator close on abrupt completion where the exception value
is not known at codegen time.
Remove CodeGenerationError and make all bytecode generation functions
return their results directly instead of wrapping them in
CodeGenerationErrorOr.
For the few remaining sites where codegen encounters an unimplemented
or unexpected AST node, we now use a new emit_todo() helper that emits
a NewTypeError + Throw sequence at compile time (preserving the runtime
behavior) and then switches to a dead basic block so subsequent codegen
for the same function can continue without issue.
This allows us to remove error handling from all callers of the
bytecode compiler, simplifying the code significantly.
Replace CodeGenerationError returns with VERIFY_NOT_REACHED() or
VERIFY() at sites that are provably unreachable:
- Non-computed member expression fallbacks in emit_load_from_reference,
emit_store_to_reference, and emit_delete_reference (member expression
properties are always computed, identifier, or private identifier)
- Two non-computed member expression fallbacks in AssignmentExpression
- Default case in compound assignment switch (all 15 AssignmentOp values
are handled)
- BindingPattern Empty/Expression name+alias pair (computed property
names always require an alias)
- Two assignment+destructuring combinations in for-in/of body evaluation
(is_destructuring is only set for VariableDeclaration lhs, which
always has VarBinding or LexicalBinding kind, never Assignment)
When a class field has a BigInt literal key like `128n = class {}`,
the anonymous class should get the name "128". The codegen path
handles Identifier, StringLiteral, and NumericLiteral keys but was
missing BigInt keys, causing the name to be empty.
Parse the BigInt literal value at codegen time and convert it to a
decimal string for both the field_name (anonymous function naming)
and class_field_initializer_name (eval("arguments") checking) paths.
Add static factory methods create_for_function_node() on
SharedFunctionInstanceData and update all callers to use them instead
of FunctionNode::ensure_shared_data().
This removes the GC::Root<SharedFunctionInstanceData> cache from
FunctionNode, eliminating the coupling between the RefCounted AST
and GC-managed runtime objects. The cache was effectively dead code:
hoisted declarations use m_functions_to_initialize directly, and
function expressions always create fresh instances during codegen.
Replace the ClassExpression const& reference in the NewClass
instruction with a u32 class_blueprint_index. The interpreter now
reads from the ClassBlueprint stored on the Executable and calls
construct_class() instead of the AST-based create_class_constructor().
Literal field initializers (numbers, booleans, null, strings, negated
numbers) are used directly in construct_class() without creating an
ECMAScriptFunctionObject, avoiding function creation overhead for
common field patterns like `x = 0` or `name = "hello"`.
Set class_field_initializer_name on SharedFunctionInstanceData at
codegen time for statically-known field keys (identifiers, private
identifiers, string literals, and numeric literals). For computed
keys, the name is set at runtime in construct_class().
ClassExpression AST nodes are no longer referenced from bytecode.
Build a ClassBlueprint from ClassExpression elements at codegen time:
- Methods/getters/setters: register SharedFunctionInstanceData from
the method's FunctionExpression
- Field initializers with literal values (numbers, booleans, null,
strings, negated numbers): store the value directly, avoiding
function creation entirely
- Field initializers with non-literal values: wrap in
ClassFieldInitializerStatement and create SharedFunctionInstanceData
- Static initializers: create SharedFunctionInstanceData from the
function body
- Constructor: register SharedFunctionInstanceData from the
constructor's FunctionExpression
Add public accessors to ClassMethod::function() and
StaticInitializer::function_body() for codegen access.
The blueprint is registered but not yet used by NewClass (dual path).
No behavioral change.
Each of the three blocks in a TryStatement (try body, catch body,
finally body) needs its own CompletionRegisterScope so that
break/continue inside any of them carries the block's own
completion value rather than leaking a value from a surrounding
statement or a different block.
Previously, statements inside these blocks would update the
enclosing scope's completion register (e.g. a for-loop's
register), and if break/continue fired with no prior expression
value, the enclosing register's stale value would leak through
as the completion value instead of undefined.
Each block now allocates a fresh register initialized to
undefined and uses it as the current completion register during
body generation. This matches the pattern already used by loops
and switch statements.
When a loop or switch body produces an abrupt completion (break or
continue) with an empty value, the ES spec requires UpdateEmpty to
replace the empty value with the last non-empty completion value V.
The bytecode compiler was failing to do this because it only updated
the completion register after body codegen, guarded by
!is_current_block_terminated(). When break/continue terminated the
block, the update was skipped.
Fix this with three changes:
1. Introduce a CompletionRegisterScope that tells
ScopeNode::generate_bytecode to eagerly emit Mov instructions
into the completion register after each value-producing
statement. This ensures the register is up to date before any
break or continue fires.
2. Give IfStatement its own CompletionRegisterScope (initialized
to undefined) during branch evaluation. This models the spec's
UpdateEmpty(stmtCompletion, undefined) for if-statements: when
break/continue fires inside an if-branch, the scoped jump
propagation sees that the if's completion register differs from
the loop's and emits a Mov, correctly replacing the eagerly
written value with undefined. Without this, code like
{ 3; if (true) { break; } else { } } would incorrectly carry
the value 3 instead of undefined through the break.
3. Capture loop body results and emit a fallback Mov for
non-ScopeNode bodies (e.g. bare expression statements like
do x=1; while(false)) that don't participate in the eager
CompletionRegisterScope update mechanism.
For labelled break/continue that cross loop boundaries, the jump
codegen now propagates the inner completion register to the target
scope's completion register before emitting the jump.
Also fix ForStatement to use a proper completion register
(previously it returned the body result directly, which was wrong
for empty bodies and break-with-no-value cases).
After replacing the runtime unwind context stack with explicit
completion records for try/finally dispatch, the distinction between
"handler" (catch) and "finalizer" (finally) in the exception handler
table is no longer meaningful at runtime.
handle_exception() checked handler first, then finalizer, but they
did the exact same thing (set the PC). When both were present, the
finalizer was dead code.
Collapse both fields into a single handler_offset (now non-optional,
since an entry always has a target), remove the finalizer concept
from BasicBlock, UnwindContext, and ExceptionHandlers, and simplify
handle_exception() to a direct assignment.
LeaveUnwindContext popped the runtime unwind context stack. With the
stack being removed, all emission sites become dead code. Remove the
opcode and all its emissions.
EnterUnwindContext pushed an UnwindInfo and jumped to entry_point.
Without the stack push, it's just a Jump. Replace the single emission
site with a Jump and remove the opcode entirely.
Replace the saved_lexical_environments stack in ExecutionContextRareData
with explicit register-based environment tracking. Environments are now
stored in registers and restored via SetLexicalEnvironment, making the
environment flow visible in bytecode.
Key changes:
- Add GetLexicalEnvironment and SetLexicalEnvironment opcodes
- CreateLexicalEnvironment takes explicit parent and dst operands
- EnterObjectEnvironment stores new environment in a dst register
- NewClass takes an explicit class_environment operand
- Remove LeaveLexicalEnvironment opcode (instead: SetLexicalEnvironment)
- Remove saved_lexical_environments from ExecutionContextRareData
- Use a reserved register for the saved lexical environment to avoid
dominance issues with lazily-emitted GetLexicalEnvironment
Each finally scope gets two registers (completion_type and
completion_value) that form an explicit completion record. Every path
into the finally body sets these before jumping, and a dispatch chain
after the finally body routes to the correct continuation.
This replaces the old implicit protocol that relied on the exception
register, a saved_return_value register, and a scheduled_jump field
on ExecutionContext, allowing us to remove:
- 5 opcodes (ContinuePendingUnwind, ScheduleJump, LeaveFinally,
RestoreScheduledJump, PrepareYield)
- 1 reserved register (saved_return_value)
- 2 ExecutionContext fields (scheduled_jump, previously_scheduled_jumps)
The spec for PropertyDefinitionEvaluation requires that when evaluating
a property definition with a computed key (PropertyDefinition :
PropertyName : AssignmentExpression), the PropertyName is fully
evaluated (including ToPropertyKey, which calls ToPrimitive) before the
value's AssignmentExpression is evaluated.
Our bytecode compiler was evaluating the key expression first, then
the value expression, and only performing ToPropertyKey later inside
PutByValue at runtime. This meant user-observable side effects from
ToPrimitive (such as calling Symbol.toPrimitive or toString on the key
object) would fire after the value expression had already been
evaluated.
Fix this by using a new ToPrimitiveWithStringHint instruction that
performs ToPrimitive with string hint(!), and emitting it between the
key and value evaluations in ObjectExpression codegen.
After ToPrimitive, the key is already a primitive, so the subsequent
ToPropertyKey inside PutByValue becomes a no-op from the perspective
of user-observable side
effects.
Also update an existing test that was asserting the old (incorrect)
evaluation order, and add comprehensive new tests for computed property
key evaluation order.
When the rest element in an object destructuring assignment targets a
MemberExpression (e.g. `({a, ...t.rest} = obj)`), we were incorrectly
storing the original source object to the reference instead of the
rest object produced by CopyObjectExcludingProperties.
For example, `({a, ...t.rest} = {a:1, b:2, c:3})` would set t.rest
to `{a:1, b:2, c:3}` instead of the correct `{b:2, c:3}`.
The fix is to pass the result of CopyObjectExcludingProperties
to emit_store_to_reference instead of the original RHS.
The FIXME comments suggested that ToPropertyKey was called at the wrong
time for computed super property access. However, extensive testing
shows that both Ladybird and V8 implement the correct ordering according
to the ECMA262 specification.
Remove the outdated FIXME comments and add comprehensive test coverage
for super property computed keys with Symbol.toPrimitive to prevent
regressions.
Route tagged template identifier lookup through
GetCalleeAndThisFromEnvironment only when the identifier is non-local.
Keep local and global identifiers on Identifier::generate_bytecode so
TDZ checks and ordinary undefined-this behavior stay intact.
Expand runtime coverage with a tagged-template TDZ regression case,
sequential with-binding calls, and getter-returned tag functions.
For non-Reference calls (e.g. (0, fn)(), (cond ? fn : x)()), the
codegen correctly passes undefined as the thisValue, matching step 2b
of EvaluateCall in the spec. OrdinaryCallBindThis then coerces
undefined to the global object in sloppy mode at runtime. Replace the
stale FIXME with a clarifying comment.
Also add comprehensive tests for this-value behavior in non-Reference
call patterns (comma, ternary, logical, assignment, nullish coalescing)
in both sloppy and strict mode.
The FIXME claimed that IsAnonymousFunctionDefinition + NamedEvaluation
was missing for simple assignment expressions like `x = function() {}`.
However, the code directly below the FIXME already implements this
correctly via emit_named_evaluation_if_anonymous_function.
There is no need to concat empty string literals when building template
literals. Now strings will only be concatenated if they need to be.
To handle the edge case where the first segment is not a string
literal, a new `ToString` op code has been added to ensure the value is
a string concatenating more strings.
In addition, basic const folding is now supported for template literal
constants (templates with no interpolated values), which is commonly
used for multi-line string constants.
This improves and expands the ability to do dead code elimination on
conditions which are always truthy or falsey.
The following cases are now optimized:
* `if (true){}` -> Only emit `if` block, ignore `else`
* `if (false){}` -> Only emit `else if`/`else` block
* `while (false){}` -> Ignore `while` loop entirely
* `for (x;false;){}` -> Only emit `x` (if it exists), skip `for` block
* Ternary -> Directly return left/right hand side if condition is const
Logical expressions like `true || false` are now constant folded. This
also allows for dead code elimination if we know the right-hand side of
the expression will never be evaluated (such as `false && f()` or
`true || f()`).
In the test suites, the values are now being constant folded at compile
time. To ensure that the actual evaluation logic is being called
properly, I had to duplicate the tests and call them via a function so
the compiler would not optimize the evaluation logic away.
This also demotes `NaN` and `Infinity` identifiers to `nan` and
`inf` double literals, which will further help with const folding.
This is a common way to convert a value to a boolean. Instead of doing
a boolean conversion and 2 negate operations, we replace this with a
single `ToBoolean` op code.
Numeric string keys like "0" are converted to numeric property keys and
stored in indexed storage rather than shape-based storage. The shape
caching optimization introduced in 505fe0a977 didn't account for this,
causing properties with numeric keys to be lost on subsequent calls.
The fix excludes object literals with numeric string keys from the
shape caching fast path by checking if any key would become a numeric
property index.
When a function creates object literals with simple property names,
we now cache the resulting shape after the first instantiation. On
subsequent calls, we create the object with the cached shape directly
and write property values at their known offsets.
This avoids repeated shape transitions and property offset lookups
for a common JavaScript pattern.
The optimization uses two new bytecode instructions:
- CacheObjectShape: Captures the final shape after object construction
- InitObjectLiteralProperty: Writes properties using cached offsets
Only "simple" object literals are optimized (string literal keys with
simple value expressions). Complex cases like computed properties,
getters/setters, and spread elements use the existing slow path.
3.4x speedup on a microbenchmark that repeatedly instantiates an object
literal with 26 properties. Small progressions on various benchmarks.
This resolves a FIXME in its code generation, particularly for:
- Caching the template object
- Setting the correct property attributes
- Freezing the resulting objects
This allows archive.org to load, which uses the Lit library.
The Lit library caches these template objects to determine if a
template has changed, allowing it to determine to do a full template
rerender or only partially update the rendering. Before, we would
always cause a full rerender on update because we didn't return the
same template object.
This caused issues with archive.org's code, I believe particularly with
its router library, where we would constantly detach and reattach nodes
unexpectedly, ending up with the page content not being attached to the
router's custom element.
Use `Op::Call` directly instead of creating a single-element array and
using `CallWithArgumentArray` when calling iterator methods (`next`,
`throw`, `return`) in `yield*` expressions.
This fixes an issue where we'd incorrectly retain objects via the
[[HomeObject]] slot. This common pattern was affected:
Object.defineProperty(o, "foo", {
get: function() { return 123; }
});
Above, the object literal would get assigned to the [[HomeObject]]
slot even though "get" is not a "method" per the spec.
This frees about 30,000 objects on my x.com home feed.
Instead of creating PropertyKeys on the fly during interpreter
execution, we now store fully-formed ones in the Executable.
This avoids a whole bunch of busywork in property access instructions
and substantially reduces code size bloat.
