Diff
Modified: trunk/LayoutTests/ChangeLog (163327 => 163328)
--- trunk/LayoutTests/ChangeLog 2014-02-03 22:39:38 UTC (rev 163327)
+++ trunk/LayoutTests/ChangeLog 2014-02-03 22:43:28 UTC (rev 163328)
@@ -1,3 +1,20 @@
+2014-02-03 Oliver Hunt <[email protected]>
+
+ Deconstructed parameters aren't being placed in the correct scope
+ https://bugs.webkit.org/show_bug.cgi?id=128126
+
+ Reviewed by Antti Koivisto.
+
+ Added tests for correct behaviour.
+
+ * js/deconstructing-parameters-should-be-locals-expected.txt: Added.
+ * js/deconstructing-parameters-should-be-locals.html: Added.
+ * js/script-tests/deconstructing-parameters-should-be-locals.js: Added.
+ (description.value.string_appeared_here.readDeconstructedParameter):
+ (overwriteDeconstructedParameter):
+ (readCapturedDeconstructedParameter):
+ (overwriteCapturedDeconstructedParameter):
+
2014-01-31 Geoffrey Garen <[email protected]>
Simplified name scope creation for function expressions
Added: trunk/LayoutTests/js/deconstructing-parameters-should-be-locals-expected.txt (0 => 163328)
--- trunk/LayoutTests/js/deconstructing-parameters-should-be-locals-expected.txt (rev 0)
+++ trunk/LayoutTests/js/deconstructing-parameters-should-be-locals-expected.txt 2014-02-03 22:43:28 UTC (rev 163328)
@@ -0,0 +1,14 @@
+This tests to ensure that ddeconstructing parameters behave like regular locals
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS readDeconstructedParameter(['inner']) is 'inner'
+PASS overwriteDeconstructedParameter(['unused']); value; is 'outer'
+PASS readCapturedDeconstructedParameter(['inner']) is 'inner'
+PASS overwriteCapturedDeconstructedParameter(['unused']); is 'innermost'
+PASS value is 'outer'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/deconstructing-parameters-should-be-locals.html (0 => 163328)
--- trunk/LayoutTests/js/deconstructing-parameters-should-be-locals.html (rev 0)
+++ trunk/LayoutTests/js/deconstructing-parameters-should-be-locals.html 2014-02-03 22:43:28 UTC (rev 163328)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/js/script-tests/deconstructing-parameters-should-be-locals.js (0 => 163328)
--- trunk/LayoutTests/js/script-tests/deconstructing-parameters-should-be-locals.js (rev 0)
+++ trunk/LayoutTests/js/script-tests/deconstructing-parameters-should-be-locals.js 2014-02-03 22:43:28 UTC (rev 163328)
@@ -0,0 +1,35 @@
+description("This tests to ensure that ddeconstructing parameters behave like regular locals")
+
+var value="outer"
+function readDeconstructedParameter([value]) {
+ return value;
+}
+
+function overwriteDeconstructedParameter([value]) {
+ value = "inner"
+}
+
+function readCapturedDeconstructedParameter([value]) {
+ return (function () {
+ return value;
+ })()
+}
+
+function overwriteCapturedDeconstructedParameter([value]) {
+ (function () {
+ value = "innermost";
+ })()
+ return value
+}
+
+shouldBe("readDeconstructedParameter(['inner'])", "'inner'")
+overwriteDeconstructedParameter(['inner'])
+
+shouldBe("overwriteDeconstructedParameter(['unused']); value;", "'outer'")
+
+shouldBe("readCapturedDeconstructedParameter(['inner'])", "'inner'")
+overwriteDeconstructedParameter(['inner'])
+
+shouldBe("overwriteCapturedDeconstructedParameter(['unused']);", "'innermost'")
+shouldBe("value", "'outer'")
+
Modified: trunk/Source/_javascript_Core/ChangeLog (163327 => 163328)
--- trunk/Source/_javascript_Core/ChangeLog 2014-02-03 22:39:38 UTC (rev 163327)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-02-03 22:43:28 UTC (rev 163328)
@@ -1,3 +1,25 @@
+2014-02-03 Oliver Hunt <[email protected]>
+
+ Deconstructed parameters aren't being placed in the correct scope
+ https://bugs.webkit.org/show_bug.cgi?id=128126
+
+ Reviewed by Antti Koivisto.
+
+ Make sure we declare the bound parameter names as variables when
+ we reparse. In the BytecodeGenerator we now also directly ensure
+ that bound parameters are placed in the symbol table of the function
+ we're currently compiling. We then delay binding until just before
+ we start codegen for the body of the function so that we can ensure
+ the function has completely initialised all scope details.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::generate):
+ (JSC::BytecodeGenerator::BytecodeGenerator):
+ * bytecompiler/BytecodeGenerator.h:
+ * parser/Parser.cpp:
+ (JSC::Parser<LexerType>::Parser):
+ (JSC::Parser<LexerType>::createBindingPattern):
+
2014-02-03 Alexey Proskuryakov <[email protected]>
Update JS whitespace definition for changes in Unicode 6.3
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (163327 => 163328)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2014-02-03 22:39:38 UTC (rev 163327)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2014-02-03 22:43:28 UTC (rev 163328)
@@ -63,6 +63,10 @@
SamplingRegion samplingRegion("Bytecode Generation");
m_codeBlock->setThisRegister(m_thisRegister.virtualRegister());
+ for (size_t i = 0; i < m_deconstructedParameters.size(); i++) {
+ auto& entry = m_deconstructedParameters[i];
+ entry.second->bindValue(*this, entry.first.get());
+ }
m_scopeNode->emitBytecode(*this);
@@ -298,10 +302,16 @@
const DeclarationStacks::FunctionStack& functionStack = functionBody->functionStack();
const DeclarationStacks::VarStack& varStack = functionBody->varStack();
+ IdentifierSet test;
// Captured variables and functions go first so that activations don't have
// to step over the non-captured locals to mark them.
if (functionBody->hasCapturedVariables()) {
+ for (size_t i = 0; i < boundParameterProperties.size(); i++) {
+ const Identifier& ident = boundParameterProperties[i];
+ if (functionBody->captures(ident))
+ addVar(ident, IsVariable, IsWatchable);
+ }
for (size_t i = 0; i < functionStack.size(); ++i) {
FunctionBodyNode* function = functionStack[i];
const Identifier& ident = function->ident();
@@ -338,6 +348,11 @@
}
}
m_lastLazyFunction = canLazilyCreateFunctions ? codeBlock->m_numVars : m_firstLazyFunction;
+ for (size_t i = 0; i < boundParameterProperties.size(); i++) {
+ const Identifier& ident = boundParameterProperties[i];
+ if (!functionBody->captures(ident))
+ addVar(ident, IsVariable, IsWatchable);
+ }
for (size_t i = 0; i < varStack.size(); ++i) {
const Identifier& ident = varStack[i].first;
if (!functionBody->captures(ident))
@@ -356,7 +371,6 @@
int nextParameterIndex = CallFrame::thisArgumentOffset();
m_thisRegister.setIndex(nextParameterIndex++);
m_codeBlock->addParameter();
- Vector<std::pair<RegisterID*, const DeconstructionPatternNode*>> deconstructedParameters;
for (size_t i = 0; i < parameters.size(); ++i, ++nextParameterIndex) {
int index = nextParameterIndex;
auto pattern = parameters.at(i);
@@ -364,7 +378,7 @@
m_codeBlock->addParameter();
RegisterID& parameter = registerFor(index);
parameter.setIndex(index);
- deconstructedParameters.append(std::make_pair(¶meter, pattern));
+ m_deconstructedParameters.append(std::make_pair(¶meter, pattern));
continue;
}
auto simpleParameter = static_cast<const BindingNode*>(pattern);
@@ -389,10 +403,6 @@
instructions().append(kill(&m_thisRegister));
instructions().append(0);
}
- for (size_t i = 0; i < deconstructedParameters.size(); i++) {
- auto& entry = deconstructedParameters[i];
- entry.second->bindValue(*this, entry.first);
- }
}
BytecodeGenerator::BytecodeGenerator(VM& vm, EvalNode* evalNode, UnlinkedEvalCodeBlock* codeBlock, DebuggerMode debuggerMode, ProfilerMode profilerMode)
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (163327 => 163328)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2014-02-03 22:39:38 UTC (rev 163327)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2014-02-03 22:43:28 UTC (rev 163328)
@@ -644,6 +644,7 @@
Vector<SwitchInfo> m_switchContextStack;
Vector<ForInContext> m_forInContextStack;
Vector<TryContext> m_tryContextStack;
+ Vector<std::pair<RefPtr<RegisterID>, const DeconstructionPatternNode*>> m_deconstructedParameters;
Vector<TryRange> m_tryRanges;
SegmentedVector<TryData, 8> m_tryData;
Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (163327 => 163328)
--- trunk/Source/_javascript_Core/parser/Parser.cpp 2014-02-03 22:39:38 UTC (rev 163327)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp 2014-02-03 22:43:28 UTC (rev 163328)
@@ -219,12 +219,26 @@
if (strictness == JSParseStrict)
scope->setStrictMode();
if (parameters) {
+ bool hadBindingParameters = false;
for (unsigned i = 0; i < parameters->size(); i++) {
auto parameter = parameters->at(i);
- if (!parameter->isBindingNode())
+ if (!parameter->isBindingNode()) {
+ hadBindingParameters = true;
continue;
+ }
scope->declareParameter(&static_cast<BindingNode*>(parameter)->boundProperty());
}
+ if (hadBindingParameters) {
+ Vector<Identifier> boundParameterNames;
+ for (unsigned i = 0; i < parameters->size(); i++) {
+ auto parameter = parameters->at(i);
+ if (parameter->isBindingNode())
+ continue;
+ parameter->collectBoundIdentifiers(boundParameterNames);
+ }
+ for (auto boundParameterName : boundParameterNames)
+ scope->declareVariable(&boundParameterName);
+ }
}
if (!name.isNull())
scope->declareCallee(&name);
@@ -498,7 +512,8 @@
}
}
if (kind != DeconstructToExpressions)
- context.addVar(&name, kind == DeconstructToParameters ? 0 : DeclarationStacks::HasInitializer);
+ context.addVar(&name, DeclarationStacks::HasInitializer);
+
} else {
if (kind == DeconstructToVariables) {
failIfFalseIfStrict(declareVariable(&name), "Cannot declare a variable named '", name.impl(), "' in strict mode");
