- Revision
- 199179
- Author
- sbar...@apple.com
- Date
- 2016-04-07 14:01:42 -0700 (Thu, 07 Apr 2016)
Log Message
Initial implementation of annex b.3.3 behavior was incorrect
https://bugs.webkit.org/show_bug.cgi?id=156276
Reviewed by Keith Miller.
Source/_javascript_Core:
I almost got annex B.3.3 correct in my first implementation.
There is a subtlety here I got wrong. We always create a local binding for
a function at the very beginning of execution of a block scope. So we
hoist function declarations to their local binding within a given
block scope. When we actually evaluate the function declaration statement
itself, we must lookup the binding in the current scope, and bind the
value to the binding in the "var" scope. We perform the following
abstract operations when executing a function declaration statement.
f = lookupBindingInCurrentScope("func")
store(varScope, "func", f)
I got this wrong by performing the store to the var binding at the beginning
of the block scope instead of when we evaluate the function declaration statement.
This behavior is observable. For example, a program could change the value
of "func" before the actual function declaration statement executes.
Consider the following two functions:
```
function foo1() {
// func === undefined
{
// typeof func === "function"
function func() { } // Executing this statement binds the local "func" binding to the implicit "func" var binding.
func = 20 // This sets the local "func" binding to 20.
}
// typeof func === "function"
}
function foo2() {
// func === undefined
{
// typeof func === "function"
func = 20 // This sets the local "func" binding to 20.
function func() { } // Executing this statement binds the local "func" binding to the implicit "func" var binding.
}
// func === 20
}
```
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::initializeBlockScopedFunctions):
(JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):
* bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::emitNodeForLeftHandSide):
* bytecompiler/NodesCodegen.cpp:
(JSC::FuncDeclNode::emitBytecode):
* tests/stress/sloppy-mode-function-hoisting.js:
(test.foo):
(test):
(test.):
(test.bar):
(test.switch.case.0):
(test.capFoo1):
(test.switch.capFoo2):
(test.outer):
(foo):
LayoutTests:
* js/function-declarations-in-switch-statement-expected.txt:
* js/script-tests/function-declarations-in-switch-statement.js:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (199178 => 199179)
--- trunk/LayoutTests/ChangeLog 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/LayoutTests/ChangeLog 2016-04-07 21:01:42 UTC (rev 199179)
@@ -1,3 +1,13 @@
+2016-04-07 Saam barati <sbar...@apple.com>
+
+ Initial implementation of annex b.3.3 behavior was incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=156276
+
+ Reviewed by Keith Miller.
+
+ * js/function-declarations-in-switch-statement-expected.txt:
+ * js/script-tests/function-declarations-in-switch-statement.js:
+
2016-04-06 Ada Chan <adac...@apple.com>
Rename TextTrackRepresentationiOS to TextTrackRepresentationCocoa and enable on Mac
Modified: trunk/LayoutTests/js/function-declarations-in-switch-statement-expected.txt (199178 => 199179)
--- trunk/LayoutTests/js/function-declarations-in-switch-statement-expected.txt 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/LayoutTests/js/function-declarations-in-switch-statement-expected.txt 2016-04-07 21:01:42 UTC (rev 199179)
@@ -3,7 +3,7 @@
WARN: shouldBe() expects string arguments
PASS 20 is 20
WARN: shouldBe() expects string arguments
-PASS 20 is 20
+PASS -1 is -1
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/js/script-tests/function-declarations-in-switch-statement.js (199178 => 199179)
--- trunk/LayoutTests/js/script-tests/function-declarations-in-switch-statement.js 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/LayoutTests/js/script-tests/function-declarations-in-switch-statement.js 2016-04-07 21:01:42 UTC (rev 199179)
@@ -21,4 +21,4 @@
shouldBe(t(1), '20');
shouldBe(t(2), '20');
-shouldBe(t(3), '20');
+shouldBe(t(3), '-1');
Modified: trunk/Source/_javascript_Core/ChangeLog (199178 => 199179)
--- trunk/Source/_javascript_Core/ChangeLog 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-04-07 21:01:42 UTC (rev 199179)
@@ -1,3 +1,67 @@
+2016-04-07 Saam barati <sbar...@apple.com>
+
+ Initial implementation of annex b.3.3 behavior was incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=156276
+
+ Reviewed by Keith Miller.
+
+ I almost got annex B.3.3 correct in my first implementation.
+ There is a subtlety here I got wrong. We always create a local binding for
+ a function at the very beginning of execution of a block scope. So we
+ hoist function declarations to their local binding within a given
+ block scope. When we actually evaluate the function declaration statement
+ itself, we must lookup the binding in the current scope, and bind the
+ value to the binding in the "var" scope. We perform the following
+ abstract operations when executing a function declaration statement.
+
+ f = lookupBindingInCurrentScope("func")
+ store(varScope, "func", f)
+
+ I got this wrong by performing the store to the var binding at the beginning
+ of the block scope instead of when we evaluate the function declaration statement.
+ This behavior is observable. For example, a program could change the value
+ of "func" before the actual function declaration statement executes.
+ Consider the following two functions:
+ ```
+ function foo1() {
+ // func === undefined
+ {
+ // typeof func === "function"
+ function func() { } // Executing this statement binds the local "func" binding to the implicit "func" var binding.
+ func = 20 // This sets the local "func" binding to 20.
+ }
+ // typeof func === "function"
+ }
+
+ function foo2() {
+ // func === undefined
+ {
+ // typeof func === "function"
+ func = 20 // This sets the local "func" binding to 20.
+ function func() { } // Executing this statement binds the local "func" binding to the implicit "func" var binding.
+ }
+ // func === 20
+ }
+ ```
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::initializeBlockScopedFunctions):
+ (JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):
+ * bytecompiler/BytecodeGenerator.h:
+ (JSC::BytecodeGenerator::emitNodeForLeftHandSide):
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::FuncDeclNode::emitBytecode):
+ * tests/stress/sloppy-mode-function-hoisting.js:
+ (test.foo):
+ (test):
+ (test.):
+ (test.bar):
+ (test.switch.case.0):
+ (test.capFoo1):
+ (test.switch.capFoo2):
+ (test.outer):
+ (foo):
+
2016-04-07 Alex Christensen <achristen...@webkit.org>
Build fix after r199170
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (199178 => 199179)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-04-07 21:01:42 UTC (rev 199179)
@@ -1934,18 +1934,30 @@
emitNewFunctionExpressionCommon(temp.get(), function);
bool isLexicallyScoped = true;
emitPutToScope(scope, variableForLocalEntry(name, entry, symbolTableIndex, isLexicallyScoped), temp.get(), DoNotThrowIfNotFound, Initialization);
+ }
+}
- if (iter->value.isSloppyModeHoistingCandidate() && m_scopeNode->hasSloppyModeHoistedFunction(name.impl())) {
- ASSERT(m_varScopeSymbolTableIndex);
- ASSERT(*m_varScopeSymbolTableIndex < m_symbolTableStack.size());
- SymbolTableStackEntry& varScope = m_symbolTableStack[*m_varScopeSymbolTableIndex];
- SymbolTable* varSymbolTable = varScope.m_symbolTable.get();
- RELEASE_ASSERT(varSymbolTable->scopeType() == SymbolTable::ScopeType::VarScope);
- SymbolTableEntry entry = varSymbolTable->get(name.impl());
- RELEASE_ASSERT(!entry.isNull());
- bool isLexicallyScoped = false;
- emitPutToScope(varScope.m_scope, variableForLocalEntry(name, entry, varScope.m_symbolTableConstantIndex, isLexicallyScoped), temp.get(), DoNotThrowIfNotFound, Initialization);
+void BytecodeGenerator::hoistSloppyModeFunctionIfNecessary(const Identifier& functionName)
+{
+ if (m_scopeNode->hasSloppyModeHoistedFunction(functionName.impl())) {
+ Variable currentFunctionVariable = variable(functionName);
+ RefPtr<RegisterID> currentValue;
+ if (RegisterID* local = currentFunctionVariable.local())
+ currentValue = local;
+ else {
+ RefPtr<RegisterID> scope = emitResolveScope(nullptr, currentFunctionVariable);
+ currentValue = emitGetFromScope(newTemporary(), scope.get(), currentFunctionVariable, DoNotThrowIfNotFound);
}
+
+ ASSERT(m_varScopeSymbolTableIndex);
+ ASSERT(*m_varScopeSymbolTableIndex < m_symbolTableStack.size());
+ SymbolTableStackEntry& varScope = m_symbolTableStack[*m_varScopeSymbolTableIndex];
+ SymbolTable* varSymbolTable = varScope.m_symbolTable.get();
+ ASSERT(varSymbolTable->scopeType() == SymbolTable::ScopeType::VarScope);
+ SymbolTableEntry entry = varSymbolTable->get(functionName.impl());
+ ASSERT(!entry.isNull());
+ bool isLexicallyScoped = false;
+ emitPutToScope(varScope.m_scope, variableForLocalEntry(functionName, entry, varScope.m_symbolTableConstantIndex, isLexicallyScoped), currentValue.get(), DoNotThrowIfNotFound, NotInitialization);
}
}
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (199178 => 199179)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2016-04-07 21:01:42 UTC (rev 199179)
@@ -478,6 +478,8 @@
return emitNode(n);
}
+ void hoistSloppyModeFunctionIfNecessary(const Identifier& functionName);
+
private:
void emitTypeProfilerExpressionInfo(const JSTextPosition& startDivot, const JSTextPosition& endDivot);
public:
Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (199178 => 199179)
--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2016-04-07 21:01:42 UTC (rev 199179)
@@ -3173,8 +3173,9 @@
// ------------------------------ FuncDeclNode ---------------------------------
-void FuncDeclNode::emitBytecode(BytecodeGenerator&, RegisterID*)
+void FuncDeclNode::emitBytecode(BytecodeGenerator& generator, RegisterID*)
{
+ generator.hoistSloppyModeFunctionIfNecessary(metadata()->ident());
}
// ------------------------------ FuncExprNode ---------------------------------
Modified: trunk/Source/_javascript_Core/tests/stress/sloppy-mode-function-hoisting.js (199178 => 199179)
--- trunk/Source/_javascript_Core/tests/stress/sloppy-mode-function-hoisting.js 2016-04-07 21:00:40 UTC (rev 199178)
+++ trunk/Source/_javascript_Core/tests/stress/sloppy-mode-function-hoisting.js 2016-04-07 21:01:42 UTC (rev 199179)
@@ -222,10 +222,21 @@
test(function() {
assert(foo === undefined);
while (truthy()) {
+ assert(foo() === 20);
break;
function foo() { return 20; }
}
+ assert(foo === undefined);
+});
+
+test(function() {
+ assert(foo === undefined);
+ while (truthy()) {
+ assert(foo() === 20);
+ function foo() { return 20; }
+ break;
+ }
assert(foo() === 20);
});
@@ -238,6 +249,18 @@
function foo() { return 20; }
}
+ assert(foo === undefined);
+ assert(bar() === undefined);
+});
+
+test(function() {
+ function bar() { return foo; }
+ assert(foo === undefined);
+ assert(bar() === undefined);
+ while (truthy()) {
+ function foo() { return 20; }
+ break;
+ }
assert(foo() === 20);
assert(bar()() === 20);
});
@@ -280,7 +303,22 @@
case 0:
function foo() { return 20; }
break;
+ case 1:
+ assert(foo() === 20);
+ break;
}
+ assert(foo === undefined);
+});
+
+test(function() {
+ assert(foo === undefined);
+ switch(1) {
+ case 1:
+ assert(foo() === 20);
+ case 0:
+ function foo() { return 20; }
+ break;
+ }
assert(foo() === 20);
});
@@ -351,6 +389,39 @@
break;
}
+ assert(foo === undefined);
+});
+
+test(function() {
+ function capFoo1() { return foo; }
+ assert(foo === undefined);
+ assert(capFoo1() === undefined);
+ switch(1) {
+ case 0:
+ function foo() { return bar; }
+ function capFoo2() { return foo; }
+ case 1:
+ let bar = 20;
+ assert(foo() === 20);
+ assert(capFoo1() === undefined);
+ assert(capFoo2() === foo);
+ assert(capFoo2()() === 20);
+ break;
+ }
+
+ assert(foo === undefined);
+});
+
+test(function() {
+ assert(foo === undefined);
+ switch(1) {
+ case 1:
+ let bar = 20;
+ assert(foo() === 20);
+ case 0:
+ function foo() { return bar; }
+ }
+
assert(foo() === 20);
});
@@ -622,6 +693,23 @@
assert(typeof z === "function");
});
+test(function() {
+ function outer() { return f; }
+ assert(outer() === undefined);
+ {
+ assert(outer() === undefined);
+ assert(f() === 2);
+ f = 100
+ assert(outer() === undefined);
+ function f() { return 1 }
+ assert(outer() === 100);
+ f = 200
+ assert(outer() === 100); // 100
+ function f() { return 2 }
+ assert(outer() === 200);
+ }
+});
+
for (let i = 0; i < 500; i++)
assert(foo() === 25);
function foo() { return 20; }
