瀏覽代碼

LibJS: Initialize functions in spec order

This is only visible with something like `Object.getOwnPropertyNames` on
the global object. All other declaration instantiations put the
functions on an environment making the order invisible.
Note that spec order is not quite tree order as in non-strict mode
functions which get hoisted out of blocks appear before top level
functions.

Co-authored-by: Hendiadyoin1 <leon.a@serenityos.org>
davidot 2 年之前
父節點
當前提交
8fa6861f66

+ 6 - 1
Userland/Libraries/LibJS/AST.cpp

@@ -4647,6 +4647,8 @@ ThrowCompletionOr<void> 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<void> 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());

+ 6 - 1
Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp

@@ -804,6 +804,8 @@ ThrowCompletionOr<void> 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<void> 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());

+ 143 - 0
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);

+ 148 - 0
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);