Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 18dd8af86932de0640de2a4586f6f3c0e72eb3c2
https://github.com/WebKit/WebKit/commit/18dd8af86932de0640de2a4586f6f3c0e72eb3c2
Author: Alexey Shvayka <[email protected]>
Date: 2023-09-22 (Fri, 22 Sep 2023)
Changed paths:
A JSTests/stress/sloppy-mode-function-hoisting-2.js
M JSTests/test262/expectations.yaml
M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
M Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
M Source/JavaScriptCore/parser/Nodes.cpp
M Source/JavaScriptCore/parser/Nodes.h
M Source/JavaScriptCore/parser/Parser.cpp
M Source/JavaScriptCore/parser/Parser.h
M Source/JavaScriptCore/parser/VariableEnvironment.h
Log Message:
-----------
[JSC] Implement lexical scope chain walk to correctly determine Annex B
hoisted functions
https://bugs.webkit.org/show_bug.cgi?id=261564
<rdar://problem/115504046>
Reviewed by Yusuke Suzuki.
Please consider the following code:
```js
{
function foo() { return 1; }
}
{
let foo = 1;
{
function foo() { return 2; }
}
}
foo(); // should be 1
```
While both functions share the same identifier, only the first one should be
hoisted per spec [1];
hoisting the second function would result in an early error due to name clash
with the lexical declaration.
Given it's impossible to determine if a function is hoistable by its identifier
only, this change replaces
maintaining identifier set of hoistable functions on a ScopeNode with a flag on
FunctionMetadataNode.
Also, this patch introduces bubbleSloppyModeFunctionHoistingCandidates() that
effectively walks up the
scope chain to ensure that for every hoisting candidate there are no clashes
with lexical bindings.
Before this change, only top-level lexical variables and parameters were
considered.
Considering the following case:
```js
{ // scope A
{ // scope B
function foo() {}
}
const foo = 1;
}
foo(); // should throw ReferenceError
```
The approach of this patch is to bubble all hoisting candidates from scope B to
A, during popScope() of B,
and then reject duplicates during popScope() of A. We can't check for clashes
during popScope() of B
because lexical declarations may be added to scope A later on.
To make this work, we must skip duplicate checks against same-scope (not
bubbled) function declarations,
which is implemented via NeedsDuplicateDeclarationCheck.
declareLexicalVariable() / declareFunction()
already check for duplicates, both in strict and sloppy mode, making this
approach viable.
When bubbling hoisting candidates, NeedsDuplicateDeclarationCheck::No can get
overriden by inner scope,
ensuring correctness of the following case [3]:
```js
{
function foo() { return 1; }
{
function foo() { return 2; }
}
}
foo(); // should be 1
```
Since per another Annex B section [2], `catch (foo) { var foo; }` doesn't
result in a SyntaxError, simple
parameter catch scopes are skipped when determining if a function should be
hoisted.
Performance-wise this change adds only a single HashMap::isEmpty() check per
scope, and was proven to be
neutral on JetStream3.
Aligns JSC with V8 and SpiderMonkey, expect for the one test [3].
[1]: https://tc39.es/ecma262/#sec-web-compat-functiondeclarationinstantiation
(29.a.ii)
[2]: https://tc39.es/ecma262/#sec-variablestatements-in-catch-blocks
[3]:
https://github.com/tc39/test262/blob/main/test/annexB/language/function-code/block-decl-nested-blocks-with-fun-decl.js
* JSTests/stress/sloppy-mode-function-hoisting-2.js: Added.
* JSTests/test262/expectations.yaml: Mark 193 tests as passing.
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h: Removes unused
m_functionOffsets.
* Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:
(JSC::FuncDeclNode::emitBytecode):
* Source/JavaScriptCore/parser/Nodes.cpp:
(JSC::ScopeNode::ScopeNode):
(JSC::ProgramNode::ProgramNode):
(JSC::ModuleProgramNode::ModuleProgramNode):
(JSC::EvalNode::EvalNode):
(JSC::FunctionMetadataNode::operator== const):
(JSC::FunctionMetadataNode::dump const):
(JSC::FunctionNode::FunctionNode):
* Source/JavaScriptCore/parser/Nodes.h:
(JSC::ScopeNode::captures):
(JSC::ScopeNode::hasSloppyModeHoistedFunction const): Deleted.
* Source/JavaScriptCore/parser/Parser.cpp:
(JSC::Parser<LexerType>::parseInner):
(JSC::Parser<LexerType>::parseFunctionDeclaration):
* Source/JavaScriptCore/parser/Parser.h:
(JSC::Scope::declareFunction):
(JSC::Scope::addSloppyModeFunctionHoistingCandidate):
(JSC::Scope::finalizeSloppyModeFunctionHoisting):
(JSC::Scope::bubbleSloppyModeFunctionHoistingCandidates):
(JSC::Scope::hasSloppyModeFunctionHoistingCandidates const):
(JSC::Parser::popScopeInternal):
(JSC::Parser::declareFunction):
(JSC::Parser<LexerType>::parse):
(JSC::Scope::addSloppyModeHoistableFunctionCandidate): Deleted.
(JSC::Scope::getSloppyModeHoistedFunctions): Deleted.
* Source/JavaScriptCore/parser/VariableEnvironment.h:
Harmonize naming to include "candidate" only when hoisting isn't guaranteed
due to unchecked clashes with lexical bindings.
Canonical link: https://commits.webkit.org/268302@main
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
