Skip to content

Commit

Permalink
[JSC] Implement Annex B block-level function hoisting for global scope
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=163209
<rdar://problem/53234438>

Reviewed by Yusuke Suzuki.

This change expands existing (and recently revamped) block-level function hoisting infrastructure
to global scope code, removing the extremely troublesome hack that used to unconditionally set the
scope of a block-level function declaration in global code to the top-level `var` scope instead of
lexical, which caused plenty of bug reports.

GlobalDeclarationInstantiation [1] implementation was tweaked to properly handle all the "phantom"
isSloppyModeHoistedFunction() variables, which are used merely to compute used / captured variables,
and not treat them like `var`s to avoid throwing spec-noncompliant errors.

Since there is no way to account for lexical declarations from another <script> in the parser [2],
nor for non-extensible global object, emitResolveScopeForHoistingFuncDeclInEval() for global code
has to emit same unresolved scope check as for declarations in eval().

Aligns JSC with the spec, V8, and SpiderMonkey.

[1]: https://tc39.es/ecma262/#sec-globaldeclarationinstantiation
[2]: https://tc39.es/ecma262/#sec-web-compat-globaldeclarationinstantiation (step 12.b.ii.2.a)

* JSTests/mozilla/ecma_3/Function/scope-001.js:
* JSTests/mozilla/js1_5/Scope/regress-184107.js:
* JSTests/mozilla/mozilla-tests.yaml:
* JSTests/stress/can-declare-global-function-invoked-before-any-func-decl-is-hoisted-eval.js:
* JSTests/stress/can-declare-global-function-invoked-before-any-func-decl-is-hoisted-global.js: Added.
* JSTests/stress/can-declare-global-var-invoked-before-any-func-decl-is-hoisted-eval.js:
* JSTests/stress/can-declare-global-var-invoked-before-any-func-decl-is-hoisted-global.js: Added.
* JSTests/stress/can-declare-global-var-invoked-during-func-decl-hoisting-eval.js:
* JSTests/stress/can-declare-global-var-invoked-during-func-decl-hoisting-global.js: Added.
* JSTests/stress/sloppy-mode-function-hoisting-global-code.js: Added.
* JSTests/stress/sloppy-mode-function-hoisting-global-code-2.js: Added.
Separate file is necessary to skip bytecode cache assertion for scripts that throw SyntaxError.
See https://commits.webkit.org/r244241.

* JSTests/stress/sloppy-mode-function-hoisting.js:
* JSTests/test262/expectations.yaml: Mark 172 tests as passing.
* LayoutTests/accessibility/insert-children-assert.html:
Adjust test that relied on previous incorrect behavior of block-level function declaration being
hoisted to top-level `var` scope before FunctionDeclaration is evaluated.

* LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html: Ditto.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-types-with-hints.tentative-expected.txt:
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):
(JSC::BytecodeGenerator::emitResolveScopeForHoistingFuncDeclInEval): Hoist the common logic between FunctionCode and EvalCode.
* Source/JavaScriptCore/parser/Parser.cpp:
(JSC::Parser<LexerType>::parseFunctionDeclarationStatement):
(JSC::Parser<LexerType>::parseFunctionDeclaration):
* Source/JavaScriptCore/parser/Parser.h:
(JSC::Parser::declareFunction):
* Source/JavaScriptCore/runtime/ProgramExecutable.cpp:
(JSC::ProgramExecutable::initializeGlobalProperties):

Canonical link: https://commits.webkit.org/268553@main
  • Loading branch information
Alexey Shvayka committed Sep 28, 2023
1 parent a13dcf3 commit a4c5231
Show file tree
Hide file tree
Showing 20 changed files with 375 additions and 447 deletions.
15 changes: 5 additions & 10 deletions JSTests/mozilla/ecma_3/Function/scope-001.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ with (obj)
}
actual = f();
}
// Mozilla result, which contradicts IE and the ECMA spec: expect = 2;
expect = 1;
expect = 2;
addThis();


Expand All @@ -99,8 +98,7 @@ with (obj)
}
}
actual = f();
// Mozilla result, which contradicts IE and the ECMA spec: expect = 2;
expect = 1;
expect = 2;
addThis();


Expand All @@ -121,8 +119,7 @@ with (obj)
}
}
actual = f();
// Mozilla result, which contradicts IE and the ECMA spec: expect = 3;
expect = 1;
expect = 3;
addThis();


Expand All @@ -142,8 +139,7 @@ with (obj)
}
delete obj;
actual = f();
// Mozilla result, which contradicts IE and the ECMA spec: expect = 2;
expect = 1;
expect = 2;
addThis();


Expand All @@ -167,8 +163,7 @@ with (obj)
{
actual = f();
}
// Mozilla result, which contradicts IE and the ECMA spec: expect = 2; // NOT 3 !!!
expect = 1;
expect = 2;
addThis();


Expand Down
3 changes: 1 addition & 2 deletions JSTests/mozilla/js1_5/Scope/regress-184107.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ addThis();

status = inSection(2);
actual = f();
// Mozilla result, which contradicts IE and the ECMA spec: expect = obj.y;
expect = y;
expect = obj.y;
addThis();

status = inSection(3);
Expand Down
2 changes: 1 addition & 1 deletion JSTests/mozilla/mozilla-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1617,7 +1617,7 @@
- path: ecma_3/FunExpr/fe-001-n.js
cmd: defaultRunMozillaTest :negative, "../shell.js"
- path: ecma_3/FunExpr/fe-001.js
cmd: defaultRunMozillaTest :fail, "../shell.js"
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma_3/FunExpr/fe-002.js
cmd: defaultRunMozillaTest :normal, "../shell.js"
- path: ecma_3/Number/15.7.4.5-1.js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ Object.defineProperty(globalThis, "foo", { get() {}, set() { fooSetCalls++; }, e

let didThrow = false;
try {
$262.evalScript(`
function foo() {}
eval(`
if (true) {
function bar() {}
}
function foo() {}
`);
} catch (err) {
didThrow = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
function assert(x) {
if (!x)
throw new Error("Bad assertion!");
}

let barSetCalls = 0;
Object.defineProperty(globalThis, "bar", { writable: false, enumerable: true, configurable: false });

let didThrow = false;
try {
$262.evalScript(`
if (true) {
function foo() {}
}
function bar() {}
`);
} catch (err) {
didThrow = true;
assert(err.toString() === "TypeError: Can't declare global function 'bar': property must be either configurable or both writable and enumerable");
}

assert(didThrow);
assert(barSetCalls === 0);
assert(!globalThis.hasOwnProperty("foo"));
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ Object.preventExtensions(globalThis);

let didThrow = false;
try {
$262.evalScript(`
var bar;
eval(`
if (true) {
function foo() {}
}
var bar;
`);
} catch (err) {
didThrow = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
function assert(x) {
if (!x)
throw new Error("Bad assertion!");
}

globalThis.bar = 1;
Object.preventExtensions(globalThis);

let didThrow = false;
try {
$262.evalScript(`
if (true) {
function bar() {}
}
var foo;
`);
} catch (err) {
didThrow = true;
assert(err.toString() === "TypeError: Can't declare global variable 'foo': global object must be extensible");
}

assert(didThrow);
assert(bar === 1);
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ function assert(x) {
throw new Error("Bad assertion!");
}

globalThis.bar = 1;
let barSetterValue;
Object.defineProperty(globalThis, "bar", { get() {}, set(val) { barSetterValue = val; }, enumerable: true, configurable: false });
Object.preventExtensions(globalThis);

eval(`{
Expand All @@ -12,4 +13,5 @@ eval(`{
}`);

assert(typeof foo === "undefined");
assert(typeof bar === "function");
assert(typeof barSetterValue === "function");
assert(!globalThis.hasOwnProperty("foo"));
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
function assert(x) {
if (!x)
throw new Error("Bad assertion!");
}

let fooSetterValue;
Object.defineProperty(globalThis, "foo", { get() {}, set(val) { fooSetterValue = val; }, enumerable: false, configurable: false });
Object.preventExtensions(globalThis);

$262.evalScript(`{
function foo() {}
function bar() {}
}`);

assert(typeof fooSetterValue === "function");
assert(typeof bar === "undefined");
assert(!globalThis.hasOwnProperty("bar"));
39 changes: 39 additions & 0 deletions JSTests/stress/sloppy-mode-function-hoisting-global-code-2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//@ runBytecodeCacheNoAssertion

function assert(x) {
if (!x)
throw new Error("Bad assertion!");
}


try {
$262.evalScript(`try {} catch ({foo2}) { function foo2() {} }`);
throw new Error("evalScript() didn't throw()!");
} catch (err) {
assert(err.toString() === "SyntaxError: Cannot declare a function that shadows a let/const/class/function variable 'foo2'.");
}
assert(!globalThis.hasOwnProperty("foo2"));


let foo9 = 9;
$262.evalScript(`{ function foo9() {} }`);
assert(foo9 === 9);
assert(!globalThis.hasOwnProperty("foo9"));


try {
$262.evalScript(`try {} catch (foo19) { function foo19() {} }`);
throw new Error("evalScript() didn't throw()!");
} catch (err) {
assert(err.toString() === "SyntaxError: Cannot declare a function that shadows a let/const/class/function variable 'foo19'.");
}
assert(!globalThis.hasOwnProperty("foo19"));


try {
$262.evalScript(`"use strict"; { function foo20() {} function foo20() {} }`);
throw new Error("evalScript() didn't throw()!");
} catch (err) {
assert(err.toString() === "SyntaxError: Cannot declare a function that shadows a let/const/class/function variable 'foo20'.");
}
assert(!globalThis.hasOwnProperty("foo19"));
163 changes: 163 additions & 0 deletions JSTests/stress/sloppy-mode-function-hoisting-global-code.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
function assert(x) {
if (!x)
throw new Error("Bad assertion!");
}


{
const MY_CONST = 1e6;
function foo1() { return MY_CONST; }
assert(foo1() === MY_CONST);
}
assert(foo1() === 1e6);


try {
throw new Error();
} catch ({foo3}) {
{ function foo3() {} }
}
assert(!globalThis.hasOwnProperty("foo3"));


with ({foo4: 4}) {
function foo5() { return foo4; }
assert(foo5() === 4);
}
assert(foo5() === 4);


with({}) {
let foo6 = 6;
function foo7() { return foo6; }
assert(foo7() === foo6);
}
assert(foo7() === 6);


let foo8 = 8;
{ function foo8 () {} }
assert(foo8 === 8);
assert(!globalThis.hasOwnProperty("foo8"));


assert(foo10 === undefined);
{
assert(foo10() === 1);
function foo10() { return 1; }
assert(foo10() === 1);
}
assert(foo10() === 1);
{
let foo10 = 1;

{
assert(foo10() === 2);
function foo10() { return 2; }
assert(foo10() === 2);
}
}
assert(foo10() === 1);


assert(foo11 === undefined);
{
assert(foo11() === 1);
function foo11() { return 1; }
assert(foo11() === 1);
}
assert(foo11() === 1);
{
{{{
assert(foo11() === 2);
function foo11() { return 2; }
assert(foo11() === 2);
}}}
let foo11 = 1;
}
assert(foo11() === 1);


assert(foo12 === undefined);
const err12 = new Error();
try {
assert(foo12() === 1);
function foo12() { return 1; }
throw err12;
} catch (foo12) {
assert(foo12 === err12);
{
assert(foo12() === 2);
function foo12() { return 2; }
assert(foo12() === 2);
}
assert(foo12 === err12);
}
assert(foo12() === 2);


assert(foo13 === undefined);
const err13 = new Error();
err13.foo13 = err13;
try {
assert(foo13() === 1);
function foo13() { return 1; }
throw err13;
} catch ({foo13}) {
assert(foo13 === err13);
{
assert(foo13() === 2);
function foo13() { return 2; }
assert(foo13() === 2);
}
assert(foo13 === err13);
}
assert(foo13() === 1);


assert(foo14 === undefined);
const err14 = new Error();
err14.foo14 = err14;
try {
assert(foo14() === 1);
function foo14() { return 1; }
throw err14;
} catch (foo14) {
assert(foo14 === err14);
{
{{
assert(foo14() === 2);
function foo14() { return 2; }
assert(foo14() === 2);
}}
const foo14 = 1;
}
assert(foo14 === err14);
}
assert(foo14() === 1);


if (true) { { function foo15() {} } } let foo15 = 15;
assert(foo15 === 15);


if (true) { function foo16() {} } let foo16 = 16;
assert(foo16 === 16);


{ if (true) function foo17() {} } let foo17 = 17;
assert(foo17 === 17);


assert(foo18 === undefined);
{
assert(foo18() === 1);
function foo18() { return 1; }
assert(foo18() === 1);
{
assert(foo18() === 2);
function foo18() { return 2; }
assert(foo18() === 2);
}
}
assert(foo18() === 1);
Loading

0 comments on commit a4c5231

Please sign in to comment.