Changeset 192603 in webkit for trunk/Source/JavaScriptCore/parser/Parser.cpp

Timestamp:

Nov 18, 2015, 5:26:36 PM (10 years ago)

Author:

[email protected]

Message:

There is a bug when default parameter values are mixed with destructuring parameter values
​https://bugs.webkit.org/show_bug.cgi?id=151369

Reviewed by Geoffrey Garen and Mark Lam.

Relanding this after a rollout.

This patch changes our parser to no longer declare destructuring
parameters as "var"s. This is a weird bug that just happened
to work in a world without default parameter values. In a world with
default parameter values this is just completely wrong. It would
incorrectly transform this program:
function foo(a = function() { b = 40; }, {b}) { a(); return b; }; foo(undefined, {b: 42}); // Should return 40
into
function foo(a = function() { b = 40; }, {b}) { var b; a(); return b; }; foo(undefined, {b:42}); // Returns 42, not 40.
Which is wrong because we end up with two distinct bindings of "b" when
there should only be one.

• parser/Parser.cpp:

(JSC::Parser<LexerType>::parseVariableDeclarationList):
(JSC::Parser<LexerType>::createBindingPattern):
(JSC::Parser<LexerType>::parseDestructuringPattern):

• parser/Parser.h:

(JSC::Scope::declareParameter):
(JSC::Scope::getUsedVariables):
(JSC::Parser::strictMode):
(JSC::Parser::isValidStrictMode):
(JSC::Parser::declareParameter):
(JSC::Parser::breakIsValid):
(JSC::Scope::declareBoundParameter): Deleted.
(JSC::Parser::declareBoundParameter): Deleted.

• tests/stress/es6-default-parameters.js:

File:

• trunk/Source/JavaScriptCore/parser/Parser.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/parser/Parser.cpp

@@ -668 +668 @@
 
 template <typename LexerType>
-template <class TreeBuilder> TreeDestructuringPattern Parser<LexerType>::createBindingPattern(TreeBuilder& context, DestructuringKind kind, ExportType exportType, const Identifier& name, int depth, JSToken token, AssignmentContext bindingContext, const Identifier** duplicateIdentifier)
+template <class TreeBuilder> TreeDestructuringPattern Parser<LexerType>::createBindingPattern(TreeBuilder& context, DestructuringKind kind, ExportType exportType, const Identifier& name, JSToken token, AssignmentContext bindingContext, const Identifier** duplicateIdentifier)
 {
     ASSERT(!name.isNull());
@@ -686 +686 @@
         }
     } else if (kind == DestructureToParameters) {
-        if (depth) {
-            auto bindingResult = declareBoundParameter(&name);
-            if (bindingResult == Scope::StrictBindingFailed && strictMode()) {
-                semanticFailIfTrue(isEvalOrArguments(&name), "Cannot destructure to a parameter name '", name.impl(), "' in strict mode");
-                if (m_lastFunctionName && name == *m_lastFunctionName)
-                    semanticFail("Cannot destructure to '", name.impl(), "' as it shadows the name of a strict mode function");
-                semanticFailureDueToKeyword("bound parameter name");
-                if (hasDeclaredParameter(name))
-                    semanticFail("Cannot destructure to '", name.impl(), "' as it has already been declared");
-                semanticFail("Cannot bind to a parameter named '", name.impl(), "' in strict mode");
-            }
-            if (bindingResult == Scope::BindingFailed) {
-                semanticFailureDueToKeyword("bound parameter name");
-                if (hasDeclaredParameter(name))
-                    semanticFail("Cannot destructure to '", name.impl(), "' as it has already been declared");
-                semanticFail("Cannot destructure to a parameter named '", name.impl(), "'");
-            }
-        } else {
-            DeclarationResultMask declarationResult = declareParameter(&name);
-            if ((declarationResult & DeclarationResult::InvalidStrictMode) && strictMode()) {
-                semanticFailIfTrue(isEvalOrArguments(&name), "Cannot destructure to a parameter name '", name.impl(), "' in strict mode");
-                if (m_lastFunctionName && name == *m_lastFunctionName)
-                    semanticFail("Cannot declare a parameter named '", name.impl(), "' as it shadows the name of a strict mode function");
-                semanticFailureDueToKeyword("parameter name");
-                if (hasDeclaredParameter(name))
-                    semanticFail("Cannot declare a parameter named '", name.impl(), "' in strict mode as it has already been declared");
-                semanticFail("Cannot declare a parameter named '", name.impl(), "' in strict mode");
-            }
-            if (declarationResult & DeclarationResult::InvalidDuplicateDeclaration) {
-                // It's not always an error to define a duplicate parameter.
-                // It's only an error when there are default parameter values or destructuring parameters.
-                // We note this value now so we can check it later.
-                if (duplicateIdentifier)
-                    *duplicateIdentifier = &name;
-            }
+        DeclarationResultMask declarationResult = declareParameter(&name);
+        if ((declarationResult & DeclarationResult::InvalidStrictMode) && strictMode()) {
+            semanticFailIfTrue(isEvalOrArguments(&name), "Cannot destructure to a parameter name '", name.impl(), "' in strict mode");
+            if (m_lastFunctionName && name == *m_lastFunctionName)
+                semanticFail("Cannot declare a parameter named '", name.impl(), "' as it shadows the name of a strict mode function");
+            semanticFailureDueToKeyword("parameter name");
+            if (hasDeclaredParameter(name))
+                semanticFail("Cannot declare a parameter named '", name.impl(), "' in strict mode as it has already been declared");
+            semanticFail("Cannot declare a parameter named '", name.impl(), "' in strict mode");
+        }
+        if (declarationResult & DeclarationResult::InvalidDuplicateDeclaration) {
+            // It's not always an error to define a duplicate parameter.
+            // It's only an error when there are default parameter values or destructuring parameters.
+            // We note this value now so we can check it later.
+            if (duplicateIdentifier)
+                *duplicateIdentifier = &name;
         }
     }
@@ -848 +829 @@
                     innerPattern = parseDestructuringPattern(context, kind, exportType, duplicateIdentifier, hasDestructuringPattern, bindingContext, depth + 1);
                 else
-                    innerPattern = createBindingPattern(context, kind, exportType, *propertyName, depth + 1, identifierToken, bindingContext, duplicateIdentifier);
+                    innerPattern = createBindingPattern(context, kind, exportType, *propertyName, identifierToken, bindingContext, duplicateIdentifier);
             } else {
                 JSTokenType tokenType = m_token.m_type;
@@ -904 +885 @@
         }
         failIfTrue(match(LET) && (kind == DestructureToLet || kind == DestructureToConst), "Can't use 'let' as an identifier name for a LexicalDeclaration");
-        pattern = createBindingPattern(context, kind, exportType, *m_token.m_data.ident, depth, m_token, bindingContext, duplicateIdentifier);
+        pattern = createBindingPattern(context, kind, exportType, *m_token.m_data.ident, m_token, bindingContext, duplicateIdentifier);
         next();
         break;