LibJS: Build functions_to_initialize in source order
ECMAScript hoisting keeps the LAST function declaration with a given name. The Rust scope_collector and script GDI extraction implemented this with a single reverse scan that pushed first-seen entries, which left the resulting list in REVERSE source order. The C++ side then iterated `m_functions_to_initialize.in_reverse()` to undo that. Switch the Rust side to a two-pass forward scan that records the last position per name and emits entries in source order, and drop the matching `.in_reverse()` calls in Script.cpp and AbstractOperations.cpp. Same hoisting semantics; NewFunction emission and global property iteration order now follow the source. The HashMap that tracks last positions is keyed on `SharedUtf16String`, so each insert is a refcount bump on the AST's existing Rc instead of a deep `Vec<u16>` clone. Add bytecode tests at script and nested-function scope that exercise multiple declarations and a duplicate name to pin the new ordering.
This commit is contained in:
parent
30394ece8d
commit
010deec578
8 changed files with 239 additions and 27 deletions
|
|
@ -997,9 +997,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, EvalDeclarationDa
|
|||
}
|
||||
|
||||
// 17. For each Parse Node f of functionsToInitialize, do
|
||||
// NB: We iterate in reverse order since we appended the functions
|
||||
// instead of prepending during pre-computation.
|
||||
for (auto const& function_to_initialize : data.functions_to_initialize.in_reverse()) {
|
||||
for (auto const& function_to_initialize : data.functions_to_initialize) {
|
||||
// a. Let fn be the sole element of the BoundNames of f.
|
||||
// b. Let fo be InstantiateFunctionObject of f with arguments lexEnv and privateEnv.
|
||||
auto function = ECMAScriptFunctionObject::create_from_function_data(
|
||||
|
|
|
|||
|
|
@ -1888,24 +1888,30 @@ fn extract_gdi_common(
|
|||
}
|
||||
}
|
||||
|
||||
// Functions to initialize (reverse order, deduplicated by name).
|
||||
let mut seen_names: HashSet<ast::Utf16String> = HashSet::new();
|
||||
let mut functions_to_init: Vec<(ast::FunctionId, ast::Utf16String)> = Vec::new();
|
||||
for child in scope.children.iter().rev() {
|
||||
// Functions to initialize: keep the last declaration with each name
|
||||
// (ECMAScript hoisting semantics), but emit them in source order. Two
|
||||
// forward passes; SharedUtf16String keys keep the inserts cheap.
|
||||
let mut last_position: std::collections::HashMap<ast::SharedUtf16String, usize> = std::collections::HashMap::new();
|
||||
for (i, child) in scope.children.iter().enumerate() {
|
||||
if let StatementKind::FunctionDeclaration(ref fd) = child.inner
|
||||
&& let Some(ref name_ident) = fd.name
|
||||
&& seen_names.insert(name_ident.name.to_utf16_string())
|
||||
{
|
||||
functions_to_init.push((fd.function_id, name_ident.name.to_utf16_string()));
|
||||
last_position.insert(name_ident.name.clone(), i);
|
||||
}
|
||||
}
|
||||
for (function_id, name) in &functions_to_init {
|
||||
let function_data = function_table.take(*function_id);
|
||||
let subtable = function_table.extract_reachable(&function_data);
|
||||
let sfd_ptr =
|
||||
unsafe { bytecode::ffi::create_sfd_for_gdi(function_data, subtable, vm_ptr, source_code_ptr, is_strict) };
|
||||
assert!(!sfd_ptr.is_null(), "create_sfd_for_gdi returned null");
|
||||
push_function(sfd_ptr, name);
|
||||
for (i, child) in scope.children.iter().enumerate() {
|
||||
if let StatementKind::FunctionDeclaration(ref fd) = child.inner
|
||||
&& let Some(ref name_ident) = fd.name
|
||||
&& last_position.get(&name_ident.name).copied() == Some(i)
|
||||
{
|
||||
let function_data = function_table.take(fd.function_id);
|
||||
let subtable = function_table.extract_reachable(&function_data);
|
||||
let sfd_ptr = unsafe {
|
||||
bytecode::ffi::create_sfd_for_gdi(function_data, subtable, vm_ptr, source_code_ptr, is_strict)
|
||||
};
|
||||
assert!(!sfd_ptr.is_null(), "create_sfd_for_gdi returned null");
|
||||
push_function(sfd_ptr, name_ident.name.as_slice());
|
||||
}
|
||||
}
|
||||
|
||||
// Var-scoped names (var VariableDeclaration names, excluding function declarations)
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@
|
|||
//! name within one scope (multiple `foo` refs are grouped together)
|
||||
|
||||
use std::cell::RefCell;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::collections::HashMap;
|
||||
use std::rc::Rc;
|
||||
|
||||
use crate::ast::{
|
||||
|
|
@ -1192,15 +1192,26 @@ impl ScopeCollector {
|
|||
let mut non_local_var_count_for_parameter_expressions: usize = 0;
|
||||
|
||||
// Build functions_to_initialize by scanning children for FunctionDeclarations.
|
||||
// Walk in reverse order, deduplicating by name.
|
||||
// ECMAScript hoisting keeps the LAST function declaration with a given name,
|
||||
// but we want to emit the resulting list in SOURCE order. Two forward passes:
|
||||
// record the last position for each name, then keep only the entries whose
|
||||
// position matches. Keys are SharedUtf16String so each insert is a cheap
|
||||
// Rc bump rather than a deep clone of the name.
|
||||
let mut functions_to_initialize: Vec<crate::ast::FunctionToInit> = Vec::new();
|
||||
let mut seen_function_names: HashSet<Utf16String> = HashSet::new();
|
||||
let mut last_position: HashMap<SharedUtf16String, usize> = HashMap::new();
|
||||
{
|
||||
let sd = scope_data.borrow();
|
||||
for i in (0..sd.children.len()).rev() {
|
||||
if let crate::ast::StatementKind::FunctionDeclaration(ref fd) = sd.children[i].inner
|
||||
for (i, child) in sd.children.iter().enumerate() {
|
||||
if let crate::ast::StatementKind::FunctionDeclaration(ref fd) = child.inner
|
||||
&& let Some(ref name_ident) = fd.name
|
||||
&& seen_function_names.insert(name_ident.name.to_utf16_string())
|
||||
{
|
||||
last_position.insert(name_ident.name.clone(), i);
|
||||
}
|
||||
}
|
||||
for (i, child) in sd.children.iter().enumerate() {
|
||||
if let crate::ast::StatementKind::FunctionDeclaration(ref fd) = child.inner
|
||||
&& let Some(ref name_ident) = fd.name
|
||||
&& last_position.get(&name_ident.name).copied() == Some(i)
|
||||
{
|
||||
functions_to_initialize.push(crate::ast::FunctionToInit { child_index: i });
|
||||
}
|
||||
|
|
@ -1215,7 +1226,7 @@ impl ScopeCollector {
|
|||
var_names.push(name.clone());
|
||||
|
||||
let is_parameter = var.flags.intersects(VarFlags::FORBIDDEN_LEXICAL);
|
||||
let is_function_name = seen_function_names.contains(name);
|
||||
let is_function_name = last_position.contains_key(name.as_slice());
|
||||
|
||||
let local_info = if let Some(ref ident) = var.var_identifier {
|
||||
if ident.is_local() {
|
||||
|
|
@ -1249,7 +1260,7 @@ impl ScopeCollector {
|
|||
vars_to_initialize.sort_by(|a, b| a.name.cmp(&b.name));
|
||||
var_names.sort();
|
||||
|
||||
if seen_function_names.iter().any(|n| n == utf16!("arguments")) {
|
||||
if last_position.contains_key(utf16!("arguments") as &[u16]) {
|
||||
has_function_named_arguments = true;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -179,9 +179,7 @@ ThrowCompletionOr<void> Script::global_declaration_instantiation(VM& vm, GlobalE
|
|||
}
|
||||
|
||||
// 16. For each Parse Node f of functionsToInitialize, do
|
||||
// NB: We iterate in reverse order since we appended the functions
|
||||
// instead of prepending during pre-computation.
|
||||
for (auto const& function_to_initialize : m_functions_to_initialize.in_reverse()) {
|
||||
for (auto const& function_to_initialize : m_functions_to_initialize) {
|
||||
// a. Let fn be the sole element of the BoundNames of f.
|
||||
// b. Let fo be InstantiateFunctionObject of f with arguments env and privateEnv.
|
||||
auto function = ECMAScriptFunctionObject::create_from_function_data(
|
||||
|
|
|
|||
74
Tests/LibJS/Bytecode/expected/function-decl-source-order.txt
Normal file
74
Tests/LibJS/Bytecode/expected/function-decl-source-order.txt
Normal file
|
|
@ -0,0 +1,74 @@
|
|||
$ae1c0998 function-decl-source-order.js:17:1
|
||||
Registers: 14
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Undefined
|
||||
|
||||
block0:
|
||||
[ 0] GetGlobal dst:reg6, `console`
|
||||
[ 18] GetById dst:reg7, base:reg6, `log` (console.log)
|
||||
[ 38] GetGlobal dst:reg9, `alpha`
|
||||
[ 50] Call dst:reg8, callee:reg9, this_value:Undefined, alpha
|
||||
[ 70] GetGlobal dst:reg10, `beta`
|
||||
[ 88] Call dst:reg9, callee:reg10, this_value:Undefined, beta
|
||||
[ a8] GetGlobal dst:reg11, `gamma`
|
||||
[ c0] Call dst:reg10, callee:reg11, this_value:Undefined, gamma
|
||||
[ e0] GetGlobal dst:reg12, `delta`
|
||||
[ f8] Call dst:reg11, callee:reg12, this_value:Undefined, delta
|
||||
[ 118] GetGlobal dst:reg13, `dup`
|
||||
[ 130] Call dst:reg12, callee:reg13, this_value:Undefined, dup
|
||||
[ 150] Call dst:reg5, callee:reg7, this_value:reg6, console.log, arguments:[reg8, reg9, reg10, reg11, reg12]
|
||||
[ 188] End value:reg5
|
||||
|
||||
|
||||
alpha$3b8324cd function-decl-source-order.js:5:20
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(1)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(1)
|
||||
|
||||
|
||||
beta$d739d7c6 function-decl-source-order.js:7:19
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(2)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(2)
|
||||
|
||||
|
||||
gamma$09aead7f function-decl-source-order.js:11:20
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(3)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(3)
|
||||
|
||||
|
||||
delta$240367f0 function-decl-source-order.js:15:20
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(4)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(4)
|
||||
|
||||
|
||||
dup$510f266c function-decl-source-order.js:13:18
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = String("second")
|
||||
|
||||
block0:
|
||||
[ 0] Return value:String("second")
|
||||
|
||||
|
||||
1 2 3 4 "second"
|
||||
|
|
@ -0,0 +1,93 @@
|
|||
$54f8ba98 nested-function-decl-source-order.js:15:1
|
||||
Registers: 11
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Undefined
|
||||
[1] = String(",")
|
||||
|
||||
block0:
|
||||
[ 0] GetGlobal dst:reg6, `console`
|
||||
[ 18] GetById dst:reg7, base:reg6, `log` (console.log)
|
||||
[ 38] GetGlobal dst:reg10, `outer`
|
||||
[ 50] Call dst:reg9, callee:reg10, this_value:Undefined, outer
|
||||
[ 70] GetById dst:reg10, base:reg9, `join`
|
||||
[ 90] Call dst:reg8, callee:reg10, this_value:reg9, <object>.join, arguments:[String(",")]
|
||||
[ b8] Call dst:reg5, callee:reg7, this_value:reg6, console.log, arguments:[reg8]
|
||||
[ e0] End value:reg5
|
||||
|
||||
|
||||
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
|
||||
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
|
||||
[ 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
|
||||
[ 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
|
||||
[ 150] NewArray dst:reg10, elements:[reg5, reg6, reg7, reg8, reg9]
|
||||
[ 178] Return value:reg10
|
||||
|
||||
|
||||
alpha$3b8324cd nested-function-decl-source-order.js:6:24
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(1)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(1)
|
||||
|
||||
|
||||
beta$d739d7c6 nested-function-decl-source-order.js:7:23
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(2)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(2)
|
||||
|
||||
|
||||
gamma$09aead7f nested-function-decl-source-order.js:9:24
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(3)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(3)
|
||||
|
||||
|
||||
delta$240367f0 nested-function-decl-source-order.js:11:24
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = Int32(4)
|
||||
|
||||
block0:
|
||||
[ 0] Return value:Int32(4)
|
||||
|
||||
|
||||
dup$510f266c nested-function-decl-source-order.js:10:22
|
||||
Registers: 5
|
||||
Blocks: 1
|
||||
Constants:
|
||||
[0] = String("second")
|
||||
|
||||
block0:
|
||||
[ 0] Return value:String("second")
|
||||
|
||||
|
||||
"1,2,3,4,second"
|
||||
17
Tests/LibJS/Bytecode/input/function-decl-source-order.js
Normal file
17
Tests/LibJS/Bytecode/input/function-decl-source-order.js
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
// Multiple top-level function declarations should be emitted in source
|
||||
// order. ECMAScript "last function with name X wins" hoisting still
|
||||
// applies, so the duplicate `dup` keeps the second body.
|
||||
|
||||
function alpha() { return 1; }
|
||||
|
||||
function beta() { return 2; }
|
||||
|
||||
function dup() { return "first"; }
|
||||
|
||||
function gamma() { return 3; }
|
||||
|
||||
function dup() { return "second"; }
|
||||
|
||||
function delta() { return 4; }
|
||||
|
||||
console.log(alpha(), beta(), gamma(), delta(), dup());
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
// Same source-order check, but for FunctionDeclarations nested inside a
|
||||
// function body. The scope_collector builds functions_to_initialize for
|
||||
// these and previously emitted them in reverse source order.
|
||||
|
||||
function outer() {
|
||||
function alpha() { return 1; }
|
||||
function beta() { return 2; }
|
||||
function dup() { return "first"; }
|
||||
function gamma() { return 3; }
|
||||
function dup() { return "second"; }
|
||||
function delta() { return 4; }
|
||||
return [alpha(), beta(), gamma(), delta(), dup()];
|
||||
}
|
||||
|
||||
console.log(outer().join(","));
|
||||
Loading…
Reference in a new issue