Assignment to new.target should be an early error
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Mar 2016 19:41:37 +0000 (19:41 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Mar 2016 19:41:37 +0000 (19:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151148

Reviewed by Mark Lam.

This patch makes it so that any form of assignment to new.target
is an early syntax error.

* parser/ASTBuilder.h:
(JSC::ASTBuilder::createNewTargetExpr):
(JSC::ASTBuilder::isNewTarget):
(JSC::ASTBuilder::createResolve):
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseAssignmentExpression):
(JSC::Parser<LexerType>::parseUnaryExpression):
* parser/SyntaxChecker.h:
(JSC::SyntaxChecker::createThisExpr):
(JSC::SyntaxChecker::createSuperExpr):
(JSC::SyntaxChecker::createNewTargetExpr):
(JSC::SyntaxChecker::isNewTarget):
(JSC::SyntaxChecker::createResolve):
(JSC::SyntaxChecker::createObjectLiteral):
* tests/es6.yaml:
* tests/stress/new-target-syntax-errors.js: Added.
(shouldBeSyntaxError):
(shouldNotBeSyntaxError):
* tests/stress/new-target.js:
(Constructor):
(doWeirdThings):
(noAssign): Deleted.
(catch): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@197947 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/parser/ASTBuilder.h
Source/JavaScriptCore/parser/Parser.cpp
Source/JavaScriptCore/parser/SyntaxChecker.h
Source/JavaScriptCore/tests/es6.yaml
Source/JavaScriptCore/tests/stress/new-target-syntax-errors.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/new-target.js

index 4a9ddf5..90107cb 100644 (file)
@@ -1,3 +1,37 @@
+2016-03-10  Saam barati  <sbarati@apple.com>
+
+        Assignment to new.target should be an early error
+        https://bugs.webkit.org/show_bug.cgi?id=151148
+
+        Reviewed by Mark Lam.
+
+        This patch makes it so that any form of assignment to new.target
+        is an early syntax error.
+
+        * parser/ASTBuilder.h:
+        (JSC::ASTBuilder::createNewTargetExpr):
+        (JSC::ASTBuilder::isNewTarget):
+        (JSC::ASTBuilder::createResolve):
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseAssignmentExpression):
+        (JSC::Parser<LexerType>::parseUnaryExpression):
+        * parser/SyntaxChecker.h:
+        (JSC::SyntaxChecker::createThisExpr):
+        (JSC::SyntaxChecker::createSuperExpr):
+        (JSC::SyntaxChecker::createNewTargetExpr):
+        (JSC::SyntaxChecker::isNewTarget):
+        (JSC::SyntaxChecker::createResolve):
+        (JSC::SyntaxChecker::createObjectLiteral):
+        * tests/es6.yaml:
+        * tests/stress/new-target-syntax-errors.js: Added.
+        (shouldBeSyntaxError):
+        (shouldNotBeSyntaxError):
+        * tests/stress/new-target.js:
+        (Constructor):
+        (doWeirdThings):
+        (noAssign): Deleted.
+        (catch): Deleted.
+
 2016-03-08  Skachkov Oleksandr  <gskachkov@gmail.com>
 
         How we load new.target in arrow functions is broken
index 3ffc16a..c24c7e0 100644 (file)
@@ -182,6 +182,7 @@ public:
         usesNewTarget();
         return new (m_parserArena) NewTargetNode(location);
     }
+    NO_RETURN_DUE_TO_CRASH bool isNewTarget(ExpressionNode*) { RELEASE_ASSERT_NOT_REACHED(); }
     ExpressionNode* createResolve(const JSTokenLocation& location, const Identifier& ident, const JSTextPosition& start, const JSTextPosition& end)
     {
         if (m_vm->propertyNames->arguments == ident)
index 63b39cc..a657909 100644 (file)
@@ -3040,6 +3040,10 @@ template <typename TreeBuilder> TreeExpression Parser<LexerType>::parseAssignmen
         }
         m_parserState.nonTrivialExpressionCount++;
         hadAssignment = true;
+        if (!TreeBuilder::CreatesAST) { // We only need to do this check with the syntax checker.
+            if (UNLIKELY(context.isNewTarget(lhs)))
+                internalFailWithMessage(false, "new.target can't be the left hand side of an assignment expression");
+        }
         context.assignmentStackAppend(assignmentStack, lhs, start, tokenStartPosition(), m_parserState.assignmentCount, op);
         start = tokenStartPosition();
         m_parserState.assignmentCount++;
@@ -4005,6 +4009,10 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseUnaryExpress
             failWithMessage("Cannot parse subexpression of ", operatorString(true, lastOperator), "operator");
         failWithMessage("Cannot parse member expression");
     }
+    if (!TreeBuilder::CreatesAST) { // We only need to do this check with the syntax checker.
+        if (UNLIKELY(lastOperator && context.isNewTarget(expr)))
+            internalFailWithMessage(false, "new.target can't come after a prefix operator");
+    }
     bool isEvalOrArguments = false;
     if (strictMode() && !m_syntaxAlreadyValidated) {
         if (context.isResolve(expr))
@@ -4013,6 +4021,10 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseUnaryExpress
     failIfTrueIfStrict(isEvalOrArguments && modifiesExpr, "Cannot modify '", m_parserState.lastIdentifier->impl(), "' in strict mode");
     switch (m_token.m_type) {
     case PLUSPLUS:
+        if (!TreeBuilder::CreatesAST) { // We only need to do this check with the syntax checker.
+            if (UNLIKELY(context.isNewTarget(expr)))
+                internalFailWithMessage(false, "new.target can't come before a postfix operator");
+        }
         m_parserState.nonTrivialExpressionCount++;
         m_parserState.nonLHSCount++;
         expr = context.makePostfixNode(location, expr, OpPlusPlus, subExprStart, lastTokenEndPosition(), tokenEndPosition());
@@ -4023,6 +4035,10 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseUnaryExpress
         next();
         break;
     case MINUSMINUS:
+        if (!TreeBuilder::CreatesAST) { // We only need to do this check with the syntax checker.
+            if (UNLIKELY(context.isNewTarget(expr)))
+                internalFailWithMessage(false, "new.target can't come before a postfix operator");
+        }
         m_parserState.nonTrivialExpressionCount++;
         m_parserState.nonLHSCount++;
         expr = context.makePostfixNode(location, expr, OpMinusMinus, subExprStart, lastTokenEndPosition(), tokenEndPosition());
index 9e6cf62..ea60a4f 100644 (file)
@@ -160,6 +160,7 @@ public:
     ExpressionType createThisExpr(const JSTokenLocation&, ThisTDZMode) { return ThisExpr; }
     ExpressionType createSuperExpr(const JSTokenLocation&) { return SuperExpr; }
     ExpressionType createNewTargetExpr(const JSTokenLocation&) { return NewTargetExpr; }
+    ALWAYS_INLINE bool isNewTarget(ExpressionType type) { return type == NewTargetExpr; }
     ExpressionType createResolve(const JSTokenLocation&, const Identifier&, int, int) { return ResolveExpr; }
     ExpressionType createObjectLiteral(const JSTokenLocation&) { return ObjectLiteralExpr; }
     ExpressionType createObjectLiteral(const JSTokenLocation&, int) { return ObjectLiteralExpr; }
index d2dd4ef..c1649e6 100644 (file)
 - path: es6/miscellaneous_RegExp_constructor_can_alter_flags.js
   cmd: runES6 :fail
 - path: es6/new.target_assignment_is_an_early_error.js
-  cmd: runES6 :fail
+  cmd: runES6 :normal
 - path: es6/non-strict_function_semantics_hoisted_block-level_function_declaration.js
   cmd: runES6 :fail
 - path: es6/Promise_is_subclassable_Promise.all.js
diff --git a/Source/JavaScriptCore/tests/stress/new-target-syntax-errors.js b/Source/JavaScriptCore/tests/stress/new-target-syntax-errors.js
new file mode 100644 (file)
index 0000000..3f880de
--- /dev/null
@@ -0,0 +1,86 @@
+function shouldBeSyntaxError(str) {
+    let failed = true;
+    try {
+        new Function(str);
+    } catch(e) {
+        if (e instanceof SyntaxError)
+            failed = false;
+    }
+    
+    if (failed)
+        throw new Error("Did not throw syntax error: " + str);
+}
+
+function shouldNotBeSyntaxError(str) {
+    let failed = false;
+    try {
+        new Function(str);
+    } catch(e) {
+        if (e instanceof SyntaxError && e.message.indexOf("new.target") !== -1)
+            failed = true;
+    }
+    
+    if (failed)
+        throw new Error("Did throw a syntax error: " + str);
+}
+
+
+let operators = ["=", "+=", "-=", "*=", "<<=", ">>=", ">>>=", "&=", "^=", "|=", "%="];
+for (let operator of operators) {
+    let functionBody = `new.target ${operator} 20`;
+    shouldBeSyntaxError(functionBody);
+
+    functionBody = `foo = new.target ${operator} 20`;
+    shouldBeSyntaxError(functionBody);
+
+    functionBody = `foo ${operator} new.target ${operator} 20`;
+    shouldBeSyntaxError(functionBody);
+
+    functionBody = `new.target ${operator} foo *= 40`;
+    shouldBeSyntaxError(functionBody);
+
+
+    // Make sure our tests cases our sound and they should not be syntax errors if new.target is replaced by foo
+    functionBody = `foo ${operator} 20`;
+    shouldNotBeSyntaxError(functionBody);
+
+    functionBody = `foo = foo ${operator} 20`;
+    shouldNotBeSyntaxError(functionBody);
+
+    functionBody = `foo ${operator} foo ${operator} 20`;
+    shouldNotBeSyntaxError(functionBody);
+
+    functionBody = `foo ${operator} foo *= 40`;
+    shouldNotBeSyntaxError(functionBody);
+}
+
+let prePostFixOperators = ["++", "--"];
+for (let operator of prePostFixOperators) {
+    let functionBody = `${operator}new.target`;
+    shouldBeSyntaxError(functionBody);
+
+    functionBody = `foo = ${operator}new.target`;
+    shouldBeSyntaxError(functionBody);
+
+    functionBody = `${operator}foo`;
+    shouldNotBeSyntaxError(functionBody);
+
+    functionBody = `foo = ${operator}foo`;
+    shouldNotBeSyntaxError(functionBody);
+}
+
+for (let operator of prePostFixOperators) {
+    let functionBody = `new.target${operator}`;
+    shouldBeSyntaxError(functionBody);
+
+    functionBody = `foo = new.target${operator}`;
+    shouldBeSyntaxError(functionBody);
+
+    functionBody = `foo${operator}`;
+    shouldNotBeSyntaxError(functionBody);
+
+    functionBody = `foo = foo${operator}`;
+    shouldNotBeSyntaxError(functionBody);
+}
+
+shouldBeSyntaxError(`({foo: new.target} = {foo:20})`);
index d7a83ff..e24664c 100644 (file)
@@ -33,21 +33,6 @@ function Constructor() {
 }
 new Constructor();
 
-function noAssign() {
-    new.target = 1;
-}
-
-try {
-    new noAssign();
-    passed = false;
-} catch(e) { }
-try {
-    noAssign();
-    passed = false;
-} catch(e) { }
-
-test(passed, true, "new.target should not be a reference");
-
 // This is mostly to test that calling new on new.target deos the right thing.
 function doWeirdThings(arg) {
     if (new.target) {