dmaax/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LibJS: Allocate local variable indices in source order

Browse source 
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.
This commit is contained in:
Andreas Kling 2026-04-26 20:47:25 +02:00 committed by Andreas Kling
parent 010deec578
commit c1bc0cdfa9
14 changed files with 94 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.lock generated
View file

@ -375,6 +375,7 @@ version = "0.1.0"
dependencies = [
"bytecode_def",
"cbindgen",
"indexmap",
"libunicode_rust",
"num-bigint",
"num-integer",

1
Libraries/LibJS/Rust/Cargo.toml
View file

@ -13,6 +13,7 @@ libunicode_rust = { path = "../../LibUnicode/Rust", default-features = false }
num-bigint = "0.4"
num-traits = "0.2"
num-integer = "0.1"
indexmap = "2"
[build-dependencies]
bytecode_def = { path = "../BytecodeDef" }

22
Libraries/LibJS/Rust/src/scope_collector.rs
View file

@ -49,6 +49,7 @@
//! - `IdentifierGroup` — a set of identifier references with the same
//! name within one scope (multiple `foo` refs are grouped together)
use indexmap::IndexMap;
use std::cell::RefCell;
use std::collections::HashMap;
use std::rc::Rc;
@ -189,8 +190,8 @@ struct ScopeRecord {
scope_level: ScopeLevel,
scope_data: Option<Rc<RefCell<ScopeData>>>,
variables: HashMap<Utf16String, ScopeVariable>,
identifier_groups: HashMap<SharedUtf16String, IdentifierGroup>,
variables: IndexMap<Utf16String, ScopeVariable>,
identifier_groups: IndexMap<SharedUtf16String, IdentifierGroup>,
functions_to_hoist: Vec<HoistableFunction>,
// Parameter tracking
@ -221,8 +222,8 @@ impl ScopeRecord {
scope_type,
scope_level,
scope_data,
variables: HashMap::new(),
identifier_groups: HashMap::new(),
variables: IndexMap::new(),
identifier_groups: IndexMap::new(),
functions_to_hoist: Vec::new(),
has_function_parameters: false,
parameter_names: Vec::new(),
@ -931,13 +932,11 @@ impl ScopeCollector {
/// - It's NOT used inside a `with` statement
/// - The scope chain is NOT poisoned by `eval()`
fn resolve_identifiers(records: &mut [ScopeRecord], index: usize, initiated_by_eval: bool, suppress_globals: bool) {
// identifier_groups is an IndexMap, so iteration is in source order
// of first reference. Local variable indices follow that order.
let groups = std::mem::take(&mut records[index].identifier_groups);
// Sort groups by name for deterministic local variable indices
// (HashMap iteration order is arbitrary).
let mut sorted_groups: Vec<_> = groups.into_iter().collect();
sorted_groups.sort_by(|a, b| a.0.cmp(&b.0));
let mut propagate_to_parent: Vec<(SharedUtf16String, IdentifierGroup)> = Vec::new();
for (name, mut group) in sorted_groups {
for (name, mut group) in groups {
// Annotate each Identifier AST node with its declaration kind,
// so the bytecode generator knows how to handle TDZ checks, etc.
if let Some(dk) = group.declaration_kind {
@ -1256,9 +1255,8 @@ impl ScopeCollector {
});
}
// Sort by name for deterministic output (HashMap iteration order is arbitrary).
vars_to_initialize.sort_by(|a, b| a.name.cmp(&b.name));
var_names.sort();
// vars_to_initialize and var_names follow source order via the
// insertion order of `record.variables`, which is now an IndexMap.
if last_position.contains_key(utf16!("arguments") as &[u16]) {
has_function_named_arguments = true;

8
Tests/LibJS/AST/expected/catch-param-scope.txt
View file

@ -34,15 +34,15 @@ Program (script) @2:1
│ │ └─ BindingPattern (object)
│ │ ├─ entry
│ │ │ └─ name
│ │ │ └─ Identifier "msg" [variable:1] @14:16
│ │ │ └─ Identifier "msg" [variable:0] @14:16
│ │ └─ entry
│ │ └─ name
│ │ └─ Identifier "code" [variable:0] @14:21
│ │ └─ Identifier "code" [variable:1] @14:21
│ └─ BlockStatement @14:29
│ └─ ReturnStatement @15:9
│ └─ BinaryExpression (+) @15:20
│ ├─ Identifier "msg" [variable:1] @15:16
│ └─ Identifier "code" [variable:0] @15:22
│ ├─ Identifier "msg" [variable:0] @15:16
│ └─ Identifier "code" [variable:1] @15:22
├─ FunctionDeclaration "catch_shadow" @20:1
│ └─ body
│ └─ FunctionBody @21:5

6
Tests/LibJS/AST/expected/class-scope.txt
View file

@ -41,7 +41,7 @@ Program (script) @2:1
│ └─ FunctionBody @20:5
│ ├─ VariableDeclaration (let) @20:5
│ │ └─ VariableDeclarator @20:5
│ │ ├─ Identifier "key" [variable:1] (let) @20:9
│ │ ├─ Identifier "key" [variable:0] (let) @20:9
│ │ └─ StringLiteral "hello" @20:22
│ ├─ ClassDeclaration @21:5
│ │ └─ ClassExpression "Bar" @21:5
@ -51,7 +51,7 @@ Program (script) @2:1
│ │ │ └─ BlockStatement @21:5
│ │ └─ elements
│ │ └─ ClassMethod @21:5
│ │ ├─ Identifier "key" [variable:1] (let) @22:10
│ │ ├─ Identifier "key" [variable:0] (let) @22:10
│ │ └─ FunctionExpression "" [strict] @22:9
│ │ └─ body
│ │ └─ FunctionBody @23:13
@ -59,7 +59,7 @@ Program (script) @2:1
│ │ └─ NumericLiteral 1 @23:20
│ └─ ReturnStatement @26:5
│ └─ NewExpression @26:12
│ └─ Identifier "Bar" [variable:0] @26:16
│ └─ Identifier "Bar" [variable:1] @26:16
└─ FunctionDeclaration "static_members" @30:1
└─ body
└─ FunctionBody @31:5

6
Tests/LibJS/AST/expected/closure-captures.txt
View file

@ -8,7 +8,7 @@ Program (script) @3:1
│ │ └─ NumericLiteral 1 @4:20
│ ├─ VariableDeclaration (let) @5:5
│ │ └─ VariableDeclarator @5:5
│ │ ├─ Identifier "not_captured" [variable:1] (let) @5:9
│ │ ├─ Identifier "not_captured" [variable:0] (let) @5:9
│ │ └─ NumericLiteral 2 @5:24
│ ├─ FunctionDeclaration "inner" @6:5
│ │ └─ body
@ -18,8 +18,8 @@ Program (script) @3:1
│ └─ ReturnStatement @9:5
│ └─ BinaryExpression (+) @9:20
│ ├─ CallExpression @9:17
│ │ └─ Identifier "inner" [variable:0] @9:12
│ └─ Identifier "not_captured" [variable:1] (let) @9:22
│ │ └─ Identifier "inner" [variable:1] @9:12
│ └─ Identifier "not_captured" [variable:0] (let) @9:22
├─ FunctionDeclaration "level0" @13:1
│ └─ body
│ └─ FunctionBody @14:5

8
Tests/LibJS/AST/expected/default-param-scope.txt
View file

@ -43,16 +43,16 @@ Program (script) @3:1
│ └─ FunctionBody @17:5
│ ├─ VariableDeclaration (var) @17:5
│ │ └─ VariableDeclarator @17:5
│ │ ├─ Identifier "v" [variable:1] (var) @17:9
│ │ ├─ Identifier "v" [variable:0] (var) @17:9
│ │ └─ Identifier "a" [argument:0] @17:13
│ ├─ VariableDeclaration (let) @18:5
│ │ └─ VariableDeclarator @18:5
│ │ ├─ Identifier "l" [variable:0] (let) @18:9
│ │ ├─ Identifier "l" [variable:1] (let) @18:9
│ │ └─ Identifier "a" [argument:0] @18:13
│ └─ ReturnStatement @19:5
│ └─ BinaryExpression (+) @19:14
│ ├─ Identifier "v" [variable:1] (var) @19:12
│ └─ Identifier "l" [variable:0] (let) @19:16
│ ├─ Identifier "v" [variable:0] (var) @19:12
│ └─ Identifier "l" [variable:1] (let) @19:16
└─ FunctionDeclaration "destruct_defaults" @23:1
├─ parameters
│ └─ BindingPattern (object)

16
Tests/LibJS/AST/expected/eval-indirect.txt
View file

@ -19,28 +19,28 @@ Program (script) @2:1
│ └─ FunctionBody @10:5
│ ├─ VariableDeclaration (let) @10:5
│ │ └─ VariableDeclarator @10:5
│ │ ├─ Identifier "x" [variable:1] (let) @10:9
│ │ ├─ Identifier "x" [variable:0] (let) @10:9
│ │ └─ NumericLiteral 1 @10:13
│ ├─ VariableDeclaration (var) @11:5
│ │ └─ VariableDeclarator @11:5
│ │ ├─ Identifier "e" [variable:0] (var) @11:9
│ │ ├─ Identifier "e" [variable:1] (var) @11:9
│ │ └─ Identifier "eval" [global] @11:13
│ ├─ ExpressionStatement @12:5
│ │ └─ CallExpression @12:6
│ │ ├─ Identifier "e" [variable:0] (var) @12:5
│ │ ├─ Identifier "e" [variable:1] (var) @12:5
│ │ └─ StringLiteral "x" @12:10
│ └─ ReturnStatement @13:5
│ └─ Identifier "x" [variable:1] (let) @13:12
│ └─ Identifier "x" [variable:0] (let) @13:12
├─ FunctionDeclaration "eval_method" @17:1
│ └─ body
│ └─ FunctionBody @18:5
│ ├─ VariableDeclaration (let) @18:5
│ │ └─ VariableDeclarator @18:5
│ │ ├─ Identifier "x" [variable:1] (let) @18:9
│ │ ├─ Identifier "x" [variable:0] (let) @18:9
│ │ └─ NumericLiteral 1 @18:13
│ ├─ VariableDeclaration (var) @19:5
│ │ └─ VariableDeclarator @19:5
│ │ ├─ Identifier "obj" [variable:0] (var) @19:9
│ │ ├─ Identifier "obj" [variable:1] (var) @19:9
│ │ └─ ObjectExpression @19:15
│ │ └─ ObjectProperty @19:15
│ │ ├─ StringLiteral "eval" @19:17
@ -48,11 +48,11 @@ Program (script) @2:1
│ ├─ ExpressionStatement @20:5
│ │ └─ CallExpression @20:13
│ │ ├─ MemberExpression @20:8
│ │ │ ├─ Identifier "obj" [variable:0] (var) @20:5
│ │ │ ├─ Identifier "obj" [variable:1] (var) @20:5
│ │ │ └─ Identifier "eval" @20:9
│ │ └─ StringLiteral "x" @20:17
│ └─ ReturnStatement @21:5
│ └─ Identifier "x" [variable:1] (let) @21:12
│ └─ Identifier "x" [variable:0] (let) @21:12
└─ FunctionDeclaration "local_eval_name" [direct-eval] [uses-this] [might-need-arguments] @25:1
└─ body
└─ FunctionBody @26:5

4
Tests/LibJS/AST/expected/object-property-key-no-capture.txt
View file

@ -4,7 +4,7 @@ Program (script) @1:1
└─ FunctionBody @2:5
├─ VariableDeclaration (var) @2:5
│ └─ VariableDeclarator @2:5
│ ├─ Identifier "x" [variable:1] (var) @2:9
│ ├─ Identifier "x" [variable:0] (var) @2:9
│ └─ NumericLiteral 1 @2:13
├─ FunctionDeclaration "inner" @3:5
│ └─ body
@ -15,4 +15,4 @@ Program (script) @1:1
│ ├─ StringLiteral "x" @4:18
│ └─ NumericLiteral 42 @4:21
└─ ReturnStatement @6:5
└─ Identifier "x" [variable:1] (var) @6:12
└─ Identifier "x" [variable:0] (var) @6:12

14
Tests/LibJS/Bytecode/expected/delete-expression-register.txt
View file

@ -14,21 +14,21 @@ block0:
f$2bc22ddd delete-expression-register.js:3:7
Registers: 7
Blocks: 3
Locals: arguments~0, x~1
Locals: x~0, arguments~1
Constants:
[0] = Undefined
[1] = Bool(false)
[2] = Int32(0)
block0:
[ 0] CreateArguments dst:arguments~0, is_immutable:false
[ 10] Mov3 dst1:x~1, src1:Undefined, dst2:x~1, src2:Bool(false), dst3:reg5, src3:x~1
[ 30] JumpTrue condition:x~1, target:block2
[ 0] CreateArguments dst:arguments~1, is_immutable:false
[ 10] Mov3 dst1:x~0, src1:Undefined, dst2:x~0, src2:Bool(false), dst3:reg5, src3:x~0
[ 30] JumpTrue condition:x~0, target:block2
block1:
[ 40] DeleteByValue dst:reg6, base:arguments~0, property:Int32(0)
[ 40] DeleteByValue dst:reg6, base:arguments~1, property:Int32(0)
[ 50] Mov dst:reg5, src:reg6
block2:
[ 60] Mov dst:x~1, src:reg5
[ 70] Return value:x~1
[ 60] Mov dst:x~0, src:reg5
[ 70] Return value:x~0

8
Tests/LibJS/Bytecode/expected/for-of-cond-rhs-block-order.txt
View file

@ -14,7 +14,7 @@ block0:
f$0adf9089 for-of-cond-rhs-block-order.js:2:21
Registers: 17
Blocks: 20
Locals: n~0, r~1
Locals: r~0, n~1
Constants:
[0] = Bool(false)
[1] = Undefined
@ -68,7 +68,7 @@ block9:
[ 1c8] Jump target:block10
block10:
[ 1d0] Mov dst:r~1, src:reg16
[ 1d0] Mov dst:r~0, src:reg16
[ 1e0] JumpFalse condition:reg12, target:block12
block11:
@ -80,11 +80,11 @@ block12:
[ 220] JumpTrue condition:reg12, target:block11
block13:
[ 230] Mov dst:n~0, src:reg16
[ 230] Mov dst:n~1, src:reg16
[ 240] JumpFalse condition:reg12, target:block15
block14:
[ 250] ThrowIfTDZ src:r~1
[ 250] ThrowIfTDZ src:r~0
[ 258] Jump target:block2
block15:

26
Tests/LibJS/Bytecode/expected/local-source-order.txt Normal file
View file

@ -0,0 +1,26 @@
$da832d31 local-source-order.js:12:1
Registers: 7
Blocks: 1
Constants:
[0] = Undefined
block0:
[ 0] GetGlobal dst:reg6, `source_order_locals`
[ 18] Call dst:reg5, callee:reg6, this_value:Undefined, source_order_locals
[ 38] End value:reg5
source_order_locals$7988cb5f local-source-order.js:6:5
Registers: 7
Blocks: 1
Locals: zebra~0, yak~1, aardvark~2
Constants:
[0] = Int32(1)
[1] = Int32(2)
[2] = Int32(3)
block0:
[ 0] Mov3 dst1:zebra~0, src1:Int32(1), dst2:yak~1, src2:Int32(2), dst3:aardvark~2, src3:Int32(3)
[ 20] Add dst:reg5, lhs:zebra~0, rhs:yak~1
[ 30] Add dst:reg6, lhs:reg5, rhs:aardvark~2
[ 40] Return value:reg6

18
Tests/LibJS/Bytecode/expected/nested-function-decl-source-order.txt
View file

@ -19,23 +19,23 @@ block0:
outer$541206d9 nested-function-decl-source-order.js:12:5
Registers: 11
Blocks: 1
Locals: alpha~0, beta~1, delta~2, dup~3, gamma~4
Locals: alpha~0, beta~1, dup~2, gamma~3, delta~4
Constants:
[0] = Undefined
block0:
[ 0] Mov3 dst1:alpha~0, src1:Undefined, dst2:beta~1, src2:Undefined, dst3:delta~2, src3:Undefined
[ 20] Mov2 dst1:dup~3, src1:Undefined, dst2:gamma~4, src2:Undefined
[ 0] Mov3 dst1:alpha~0, src1:Undefined, dst2:beta~1, src2:Undefined, dst3:dup~2, src3:Undefined
[ 20] Mov2 dst1:gamma~3, src1:Undefined, dst2:delta~4, src2:Undefined
[ 38] NewFunction dst:alpha~0, shared_function_data_index:0
[ 50] NewFunction dst:beta~1, shared_function_data_index:1
[ 68] NewFunction dst:gamma~4, shared_function_data_index:2
[ 80] NewFunction dst:dup~3, shared_function_data_index:3
[ 98] NewFunction dst:delta~2, shared_function_data_index:4
[ 68] NewFunction dst:gamma~3, shared_function_data_index:2
[ 80] NewFunction dst:dup~2, shared_function_data_index:3
[ 98] NewFunction dst:delta~4, shared_function_data_index:4
[ b0] Call dst:reg5, callee:alpha~0, this_value:Undefined, alpha
[ d0] Call dst:reg6, callee:beta~1, this_value:Undefined, beta
[ f0] Call dst:reg7, callee:gamma~4, this_value:Undefined, gamma
[ 110] Call dst:reg8, callee:delta~2, this_value:Undefined, delta
[ 130] Call dst:reg9, callee:dup~3, this_value:Undefined, dup
[ f0] Call dst:reg7, callee:gamma~3, this_value:Undefined, gamma
[ 110] Call dst:reg8, callee:delta~4, this_value:Undefined, delta
[ 130] Call dst:reg9, callee:dup~2, this_value:Undefined, dup
[ 150] NewArray dst:reg10, elements:[reg5, reg6, reg7, reg8, reg9]
[ 178] Return value:reg10

12
Tests/LibJS/Bytecode/input/local-source-order.js Normal file
View file

@ -0,0 +1,12 @@
// Local variables should be allocated indices in source order, not in
// alphabetical order. With names that sort in reverse alphabetical
// order, the difference is visible in the bytecode dump (~0 first, etc).
function source_order_locals() {
let zebra = 1;
let yak = 2;
let aardvark = 3;
return zebra + yak + aardvark;
}
source_order_locals();