diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index b07b6c09e8..a128568f11 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -4647,6 +4647,8 @@ ThrowCompletionOr Program::global_declaration_instantiation(Interpreter& i // Note: Already done in step iv. above. // 4. Insert d as the first element of functionsToInitialize. + // NOTE: Since prepending is much slower, we just append + // and iterate in reverse order in step 16 below. functions_to_initialize.append(function); return {}; })); @@ -4757,7 +4759,10 @@ ThrowCompletionOr Program::global_declaration_instantiation(Interpreter& i })); // 16. For each Parse Node f of functionsToInitialize, do - for (auto& declaration : functions_to_initialize) { + // NOTE: We iterate in reverse order since we appended the functions + // instead of prepending. We append because prepending is much slower + // and we only use the created vector here. + for (auto& declaration : functions_to_initialize.in_reverse()) { // 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(realm, declaration.name(), declaration.source_text(), declaration.body(), declaration.parameters(), declaration.function_length(), &global_environment, private_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object(), declaration.contains_direct_call_to_eval()); diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp index 1f108dca5d..39abfd7172 100644 --- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp @@ -804,6 +804,8 @@ ThrowCompletionOr eval_declaration_instantiation(VM& vm, Program const& pr // Note: Already done in step iv. // 3. Insert d as the first element of functionsToInitialize. + // NOTE: Since prepending is much slower, we just append + // and iterate in reverse order in step 17 below. functions_to_initialize.append(function); return {}; })); @@ -958,7 +960,10 @@ ThrowCompletionOr eval_declaration_instantiation(VM& vm, Program const& pr })); // 17. For each Parse Node f of functionsToInitialize, do - for (auto& declaration : functions_to_initialize) { + // NOTE: We iterate in reverse order since we appended the functions + // instead of prepending. We append because prepending is much slower + // and we only use the created vector here. + for (auto& declaration : functions_to_initialize.in_reverse()) { // 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(realm, declaration.name(), declaration.source_text(), declaration.body(), declaration.parameters(), declaration.function_length(), lexical_environment, private_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object()); diff --git a/Userland/Libraries/LibJS/Tests/syntax/functions-in-tree-order-non-strict.js b/Userland/Libraries/LibJS/Tests/syntax/functions-in-tree-order-non-strict.js new file mode 100644 index 0000000000..baa7553636 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/syntax/functions-in-tree-order-non-strict.js @@ -0,0 +1,143 @@ +// Note: This must be checked at script level because that is the only place +// where function order is visible. We introduce some test(...) with +// names to make sure the file does have some tests and a suite. + +const evalViaArrow = x => eval(x); +const evalRef = eval; +const expectedBeforeEval = "1256MNQR3478CDHIOP"; +const expectedAfterEval = "1256MNQR3478CDHIOPA9B"; +const expectedAfterEvalRef = "1256MNQR3478CDHIOPA9BKJL"; +const expectOrderToBe = expectedOrder => { + const currentOrder = Object.getOwnPropertyNames(this) + .filter(s => s.length === 2 && s[0] === "f") + .map(s => s[1]) + .join(""); + expect(currentOrder).toBe(expectedOrder); +}; + +test("function order should be in tree order and nothing in eval should be included", () => { + expectOrderToBe(expectedBeforeEval); +}); + +{ + function f1() {} + + expectOrderToBe(expectedBeforeEval); + + function f2() {} +} + +expectOrderToBe(expectedBeforeEval); + +function f3() {} + +expectOrderToBe(expectedBeforeEval); + +function f4() {} + +expectOrderToBe(expectedBeforeEval); + +{ + function f5() {} + + function f6() {} +} + +function f7() {} + +function f8() {} + +expectOrderToBe(expectedBeforeEval); +eval(` + expectOrderToBe(expectedAfterEval); + + function f9() {} + + { + function fA() {} + } + + function fB() {} + + expectOrderToBe(expectedAfterEval); +`); + +expectOrderToBe(expectedAfterEval); + +function fC() {} + +function fD() {} + +expectOrderToBe(expectedAfterEval); + +// This eval does not do anything because it goes via a function, this means +// its parent environment is not the global environment so it does not have +// a global var environment and does not put the functions on `this`. +evalViaArrow(` + expectOrderToBe(expectedAfterEval); + + function fE() {} + + { + expectOrderToBe(expectedAfterEval); + function fF() {} + } + + function fG() {} + + expectOrderToBe(expectedAfterEval); +`); + +test("function order should be in tree order, functions in eval should be in order but at the back", () => { + expectOrderToBe(expectedAfterEval); +}); + +function fH() {} + +function fI() {} + +expectOrderToBe(expectedAfterEval); + +// This is an indirect eval, but still has the global scope as immediate +// parent so it does influence the global `this`. +evalRef(` + expectOrderToBe(expectedAfterEvalRef); + console.log(2, JSON.stringify(Object.getOwnPropertyNames(this).filter(s => s.length === 2))); + + function fJ() {} + + { + expectOrderToBe(expectedAfterEvalRef); + function fK() {} + } + + function fL() {} + + expectOrderToBe(expectedAfterEvalRef); +`); + +{ + function fM() {} + + function fN() {} +} + +test("function order should be in tree order, functions in evalRef should be in order but at the back", () => { + expectOrderToBe(expectedAfterEvalRef); +}); + +function fO() {} + +function fP() {} + +{ + function fQ() {} + + { + expectOrderToBe(expectedAfterEvalRef); + } + + function fR() {} +} + +expectOrderToBe(expectedAfterEvalRef); diff --git a/Userland/Libraries/LibJS/Tests/syntax/functions-in-tree-order-strict.js b/Userland/Libraries/LibJS/Tests/syntax/functions-in-tree-order-strict.js new file mode 100644 index 0000000000..aaa6ead065 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/syntax/functions-in-tree-order-strict.js @@ -0,0 +1,148 @@ +"use strict"; +// Note: This must be checked at script level because that is the only place +// where function order is visible. We introduce some test(...) with +// names to make sure the file does have some tests and a suite. + +// Note: In strict mode we expect almost the same result except that the +// functions in blocks do not get hoisted up. +// Only the indirect eval copy gets the global var environment. + +const evalViaArrow = x => eval(x); +const evalRef = eval; +const expectedBeforeEval = "3478CDHIOP"; +const expectedAfterEval = "3478CDHIOP"; +const expectedAfterEvalRef = "3478CDHIOPKJL"; +const expectOrderToBe = expectedOrder => { + const currentOrder = Object.getOwnPropertyNames(this) + .filter(s => s.length === 2 && s[0] === "f") + .map(s => s[1]) + .join(""); + expect(currentOrder).toBe(expectedOrder); +}; + +test("in strict mode: function order should be in tree order and nothing in eval should be included, in strict mode functions should not be hoisted", () => { + expectOrderToBe(expectedBeforeEval); +}); + +{ + function f1() {} + + expectOrderToBe(expectedBeforeEval); + + function f2() {} +} + +expectOrderToBe(expectedBeforeEval); + +function f3() {} + +expectOrderToBe(expectedBeforeEval); + +function f4() {} + +expectOrderToBe(expectedBeforeEval); + +{ + function f5() {} + + function f6() {} +} + +function f7() {} + +function f8() {} + +expectOrderToBe(expectedBeforeEval); +eval(` + expectOrderToBe(expectedAfterEval); + + function f9() {} + + { + function fA() {} + } + + function fB() {} + + expectOrderToBe(expectedAfterEval); +`); + +expectOrderToBe(expectedAfterEval); + +function fC() {} + +function fD() {} + +expectOrderToBe(expectedAfterEval); + +// This eval does not do anything because it goes via a function, this means +// its parent environment is not the global environment so it does not have +// a global var environment and does not put the functions on `this`. +evalViaArrow(` + expectOrderToBe(expectedAfterEval); + + function fE() {} + + { + expectOrderToBe(expectedAfterEval); + function fF() {} + } + + function fG() {} + + expectOrderToBe(expectedAfterEval); +`); + +test("in strict mode: function order should be in tree order, functions in eval should be in order but at the back, in strict mode functions should not be hoisted", () => { + expectOrderToBe(expectedAfterEval); +}); + +function fH() {} + +function fI() {} + +expectOrderToBe(expectedAfterEval); + +// This is an indirect eval, but still has the global scope as immediate +// parent so it does influence the global `this`. +evalRef(` + expectOrderToBe(expectedAfterEvalRef); + console.log(2, JSON.stringify(Object.getOwnPropertyNames(this).filter(s => s.length === 2))); + + function fJ() {} + + { + expectOrderToBe(expectedAfterEvalRef); + function fK() {} + } + + function fL() {} + + expectOrderToBe(expectedAfterEvalRef); +`); + +{ + function fM() {} + + function fN() {} +} + +test("in strict mode: function order should be in tree order, functions in evalRef should be in order but at the back, in strict mode functions should not be hoisted", () => { + expectOrderToBe(expectedAfterEvalRef); +}); + +function fO() {} + +function fP() {} + +{ + function fQ() {} + + { + expectOrderToBe(expectedAfterEvalRef); + } + + function fR() {} +} + +expectOrderToBe(expectedAfterEvalRef);