Functions created via new Function() cannot assume that unresolved
identifiers refer to global variables, since they may be called in
an arbitrary scope. Pass a flag through the scope collector analysis
to suppress the global identifier optimization in this case.
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.
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.
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.
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.
Label scoping is already managed manually at function and arrow
function boundaries via move + ScopeGuard, and speculative parse
paths never modify labels before their potential failure points.
Move the HashMap to a direct Parser member so it is no longer
copied on every save_state()/load_state() cycle.
Replace the HashMap<size_t, Position> used to communicate invalid
object literal property ranges from parse_object_expression() to
parse_expression() with a field on PrimaryExpressionParseResult.
The range now flows through the parser's return values instead of
being stashed in persistent ParserState, eliminating a HashMap that
was never cleared and got bulk-copied on every save_state() call
during speculative parsing.
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.
The parser previously detected direct eval() calls at the end of
parse_expression(), by checking if the final expression was a
CallExpression with "eval" as the callee. This missed cases where
eval() appeared as a subexpression, e.g. `eval(code) | 0`, since
the final expression would be a BinaryExpression, not a
CallExpression.
Move the detection into parse_call_expression() where the
CallExpression is actually created. This ensures we always set the
contains_direct_call_to_eval flag regardless of surrounding
operators, so local variables are correctly placed in the
declarative environment where eval'd code can find them.
Otherwise we'll fail to recognise that the identifier token is invalid
(e.g. a keyword such as null) in a binding pattern and that it may not
be a binding pattern after all, but an object expression.
Fixes some scripts on Discord failing to parse.
Previously, when parsing a named function expression like
`Oops = function Oops() { Oops }`, the parser set a group-level flag
`might_be_variable_in_lexical_scope_in_named_function_assignment` that
propagated to the parent scope. This incorrectly prevented ALL `Oops`
identifiers from being marked as global, including those outside the
function expression.
Fix this by marking identifiers individually using
`set_is_inside_scope_with_eval()` only for identifiers inside the
function scope. This allows identifiers outside the function expression
to correctly use GetGlobal/SetGlobal while identifiers inside still
use GetBinding (since they may refer to the function's name binding).
Previously, when a nested function contained eval(), the parser would
mark all identifiers in parent functions as "inside scope with eval".
This prevented those identifiers from being marked as global, forcing
them to use GetBinding instead of GetGlobal.
However, eval() can only inject variables into its containing function's
scope, not into parent function scopes. So a parent function's reference
to a global like `Number` should still be able to use GetGlobal even if
a nested function contains eval().
This change adds a new flag `m_eval_in_current_function` that propagates
through block scopes within the same function but stops at function
boundaries. This flag is used for marking identifiers, while the
existing `m_screwed_by_eval_in_scope_chain` continues to propagate
across functions for local variable deoptimization (since eval can
access closure variables).
Before: `new Number(42)` in outer() with eval in inner() -> GetBinding
After: `new Number(42)` in outer() with eval in inner() -> GetGlobal
Cache necessary data during parsing to eliminate HashMap operations
in SharedFunctionInstanceData construction.
Before: 2 HashMap copies + N HashMap insertions with hash computations
After: Direct vector iteration with no hashing
Build FunctionScopeData for function scopes in the parser containing:
- functions_to_initialize: deduplicated var-scoped function decls
- vars_to_initialize: var decls with is_parameter/is_function_name
- var_names: HashTable for AnnexB extension checks
- Pre-computed counts for environment size calculation
- Flags for "arguments" handling
Add ScopeNode::ensure_function_scope_data() to compute the data
on-demand for edge cases that don't go through normal parser flow
(synthetic class constructors, static initializers, module wrappers).
Use this cached data directly in SFID with zero HashMap operations.
The previous fix prevented eval() in sibling function scopes from
affecting each other, but it still had a limitation: when identifiers
from multiple scopes were merged into the same identifier group at
Program scope, the presence of eval() anywhere would taint all
identifiers in the group.
This change tracks per-identifier whether it was inside a scope with
eval() in the scope chain. When a scope closes, if it contains eval()
or has eval() in its parent chain, each identifier in that scope is
marked with `is_inside_scope_with_eval`. At Program scope finalization,
only identifiers that are NOT marked can be optimized to global lookups.
This allows code like:
```js
var x = undefined; // Can be optimized (program scope)
(function() {
function walk() { undefined; } // Cannot be optimized
eval('');
})();
```
Before: Neither `undefined` could be optimized
After: The program-scope `undefined` is optimized, while the one inside
the function with eval() correctly uses dynamic lookup.
The parser was preventing identifiers like `undefined` from being
optimized to global constants when eval() was present in any function
scope, even when eval() could not affect the identifier's binding.
This change makes the parser smarter by preventing eval() in one
function scope from affecting global identifier optimization in sibling
function scopes. The key insight is that eval() in function A cannot
shadow globals in function B.
Instead, let functions have a view into the AST's SourceCode object's
underlying string data. The source string is kept alive by the AST, so
it's fine to have views into it as long as the AST exists.
Reduces memory footprint on my x.com home feed by 65 MiB.
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.
The lookahead lexer used by next_token() no longer needs to be kept
alive, since tokens created by Parser::next_token() now have any string
views guaranteed safe by the fact that they point into the one true
SourceCode provided by whoever set up the lexer.
This moves the responsibility of setting up a SourceCode object to the
users of JS::Lexer.
This means Lexer and Parser are free to use string views into the
SourceCode internally while working.
It also means Lexer no longer has to think about anything other than
UTF-16 (or ASCII) inputs. So the unit test for parsing various invalid
UTF-8 sequences is deleted here.
This commits puts the strict mode flag in the header of every bytecode
instruction. This allows us to check for strict mode without looking at
the currently running execution context.
This allows us to use the GetGlobal and SetGlobal bytecode instructions
for them, enabling cached accesses.
2.62x speed-up on this Fibonacci program:
function fib(n) {
return n < 2 ? n : fib(n - 1) + fib(n - 2);
}
for (let i = 0; i < 50_000; ++i)
fib(10);
This ports the lexer to UTF-16 and deals with the immediate fallout up
to the AST. The AST will be dealt with in upcoming commits.
The lexer will still accept UTF-8 strings as input, and will transcode
them to UTF-16 for lexing. This doesn't actually incur a new allocation,
as we were already converting the input StringView to a ByteString for
each lexer.
One immediate logical benefit here is that we do not need to know off-
hand how many UTF-8 bytes some special code points occupy. They all
happen to be a single UTF-16 code unit. So instead of advancing the
lexer by 3 positions in some cases, we can just always advance by 1.
Currently, the lexer holds a ByteString, which is always heap-allocated.
When we create a copy of the lexer for the lookahead token, that token
will outlive the lexer copy. The token holds a couple of string views
into the lexer's source string. This is fine for now, because the source
string will be kept alive by the original lexer.
But if the lexer were to hold a String or Utf16String, short strings
will be stored on the stack due to SSO. Thus the token will hold views
into released stack data. We need to keep the lookahead lexer alive to
prevent UAF on views into its source string.
This reverts commit c14173f651. We
should only annotate the minimum number of symbols that external
consumers actually use, so I am starting from scratch to do that
Instead, we can just use the scope type to determine if a scope is a
function scope.
This fixes using `this` for parameter default values in arrow functions
crashing. This happened by `uses_this_from_environment` was not set in
`set_uses_this`, as it didn't think it was in a function scope whilst
parsing parameters.
Fixes closing modal dialogs causing a crash on https://www.ikea.com/
No test262 diff.
Reverts the functional part of 08cfd5f, because it was a workaround for
this issue.
In the following example:
```js
const f = (i) => ({
obj: { a: { x: i }, b: { x: i } },
g: () => {},
});
```
The body of function `f` is initially parsed as an arrow function. As a
result, what is actually an object expression is interpreted as a formal
parameter with a binding pattern. Since duplicate identifiers are not
allowed in this context (`i` in the example), the parser generates an
error, causing the entire script to fail parsing.
This change ignores the "Duplicate parameter names in bindings" error
during arrow function parameter parsing, allowing the parser to continue
and recognize the object expression of the outer arrow function with an
implicit return.
Fixes error on https://chat.openai.com/
`var` bindings are never in the temporal dead zone (TDZ), and so we
know accessing them will not throw.
We now take advantage of this by having a specialized environment
binding value getter that doesn't check for exceptional cases.
1.08x speedup on JetStream.
This allows us to get rid of instructions that move arguments to locals
and allocate smaller JS::Value vector in ExecutionContext by reusing
slots that were already allocated for arguments.
With this change for following function:
```js
function f(x, y) {
return x + y;
}
```
we now produce following bytecode:
```
[ 0] 0: Add dst:reg6, lhs:arg0, rhs:arg1
[ 10] Return value:reg6
```
instead of:
```
[ 0] 0: GetArgument 0, dst:x~1
[ 10] GetArgument 1, dst:y~0
[ 20] Add dst:reg6, lhs:x~1, rhs:y~0
[ 30] Return value:reg6
```
Previously we blocked using locals for function arguments whenever
`arguments` was mentioned in function body, however, this is not
necessary in strict mode, where mutations to the arguments object are
not reflected in the function arguments and vice versa.
By doing that we consistently use Identifier node for identifiers and
also enable mechanism that registers identifiers in a corresponding
ScopePusher for catch parameters, which is necessary for work in the
upcoming changes.
