LibJS: Preserve outer parameter bindings for arrow arguments access#8883
LibJS: Preserve outer parameter bindings for arrow arguments access#8883LucaLeukert wants to merge 1 commit intoLadybirdBrowser:masterfrom
Conversation
Propagate arguments-object usage through arrow-function scopes during scope analysis. This keeps mapped outer parameters in the environment when an arrow function reads from the enclosing function's arguments object, such as in `(() => arguments[0])()`.
There was a problem hiding this comment.
Pull request overview
Fixes a LibJS scope-analysis bug where an arrow function’s access to the enclosing function’s arguments object could allow the outer function’s mapped parameter binding to be optimized away, leading to a crash.
Changes:
- Adjusts scope-flag propagation so
argumentsaccess from arrow functions can be attributed to the enclosing function scope. - Adds a runtime regression test covering
(() => arguments[0])()inside an outer function.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Tests/LibJS/Runtime/arguments-object.js | Adds a regression test for arrow access to an enclosing arguments object. |
| Libraries/LibJS/Rust/src/scope_collector.rs | Updates scope closing/flag propagation logic to preserve outer parameter bindings when arrows access arguments. |
Comments suppressed due to low confidence (1)
Libraries/LibJS/Rust/src/scope_collector.rs:461
close_scope()now propagatescontains_await_expressionout of arrow-function scopes. This will incorrectly mark module code likeasync () => await 1as having top-level await, since the await occurs inside a function. Consider keeping function scopes (including arrows) as a boundary forcontains_await_expressionpropagation, and only special-casing arrow functions for propagating thearguments-access flag.
if let Some(parent_index) = self.records[index].parent
&& (self.records[index].scope_type != ScopeType::Function
|| self.records[index].is_arrow_function)
{
let c = &self.records[index];
let arguments = c.contains_access_to_arguments_object_in_non_strict_mode;
let eval = c.contains_direct_call_to_eval;
let contains_await = c.contains_await_expression;
self.records[parent_index].contains_access_to_arguments_object_in_non_strict_mode |=
arguments;
self.records[parent_index].contains_direct_call_to_eval |= eval;
self.records[parent_index].contains_await_expression |= contains_await;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return (() => arguments[0])(); | ||
| } | ||
|
|
||
| expect(outer(42)).toBe(42); |
There was a problem hiding this comment.
This regression test validates the crash fix, but it doesn't actually assert mapped-arguments behavior (an unmapped arguments object would still return the original value). To ensure the outer parameter binding is preserved and mapped, add an assertion that mutating arguments[0] (or the parameter) inside the arrow is reflected in the other binding.
| return (() => arguments[0])(); | |
| } | |
| expect(outer(42)).toBe(42); | |
| return (() => { | |
| expect(arguments[0]).toBe(value); | |
| arguments[0] = 84; | |
| return value; | |
| })(); | |
| } | |
| expect(outer(42)).toBe(84); |
When an arrow function accesses the enclosing function's arguments object,
LibJS must keep mapped parameter bindings in the outer environment.
This fixes a case where
(() => arguments[0])()caused the outerparameter binding to be optimized away, which then broke mapped
arguments access.
Added a regression test for that case.
Closes #8805