We incorrectly parse arrow function expressions
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Apr 2016 15:21:51 +0000 (15:21 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Apr 2016 15:21:51 +0000 (15:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156373

Reviewed by Mark Lam.

Source/JavaScriptCore:

This patch removes the notion of "isEndOfArrowFunction".
This was a very weird function and it was incorrect.
It checked that the arrow functions with concise body
grammar production "had a valid ending". "had a valid
ending" is in quotes because concise body arrow functions
have a valid ending as long as their body has a valid
assignment expression. I've removed all notion of this
function because it was wrong and was causing us
to throw syntax errors on valid programs.

* parser/Lexer.cpp:
(JSC::Lexer<T>::nextTokenIsColon):
(JSC::Lexer<T>::lex):
(JSC::Lexer<T>::setTokenPosition): Deleted.
* parser/Lexer.h:
(JSC::Lexer::setIsReparsingFunction):
(JSC::Lexer::isReparsingFunction):
(JSC::Lexer::lineNumber):
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseInner):
(JSC::Parser<LexerType>::parseArrowFunctionSingleExpressionBodySourceElements):
(JSC::Parser<LexerType>::parseFunctionInfo):
* parser/Parser.h:
(JSC::Parser::matchIdentifierOrKeyword):
(JSC::Parser::tokenStart):
(JSC::Parser::autoSemiColon):
(JSC::Parser::canRecurse):
(JSC::Parser::isEndOfArrowFunction): Deleted.
(JSC::Parser::setEndOfStatement): Deleted.
* tests/stress/arrowfunction-others.js:
(testCase):
(simpleArrowFunction):
(truthy):
(falsey):

LayoutTests:

* js/parser-syntax-check-expected.txt:
* js/script-tests/parser-syntax-check.js:
(catch):

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

LayoutTests/ChangeLog
LayoutTests/js/parser-syntax-check-expected.txt
LayoutTests/js/script-tests/parser-syntax-check.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/parser/Lexer.cpp
Source/JavaScriptCore/parser/Lexer.h
Source/JavaScriptCore/parser/Parser.cpp
Source/JavaScriptCore/parser/Parser.h
Source/JavaScriptCore/tests/stress/arrowfunction-others.js

index 12ea625..5e2d704 100644 (file)
@@ -1,3 +1,14 @@
+2016-04-12  Saam barati  <sbarati@apple.com>
+
+        We incorrectly parse arrow function expressions
+        https://bugs.webkit.org/show_bug.cgi?id=156373
+
+        Reviewed by Mark Lam.
+
+        * js/parser-syntax-check-expected.txt:
+        * js/script-tests/parser-syntax-check.js:
+        (catch):
+
 2016-03-17  Sergio Villar Senin  <svillar@igalia.com>
 
         [css-grid] Add parsing support for <auto-repeat> syntax
index a9a4a4e..06e30a0 100644 (file)
@@ -1231,6 +1231,34 @@ PASS Invalid: "foo(x ==> x)"
 PASS Invalid: "function f() { foo(x ==> x) }"
 PASS Invalid: "foo(x = x ==> x)"
 PASS Invalid: "function f() { foo(x = x ==> x) }"
+PASS Valid:   "var f = cond ? ()=>20 : ()=>20" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? ()=>20 : ()=>20 }"
+PASS Valid:   "var f = cond ? (x)=>{x} : ()=>20" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? (x)=>{x} : ()=>20 }"
+PASS Valid:   "var f = cond ? (x)=>x : ()=>{2}" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? (x)=>x : ()=>{2} }"
+PASS Valid:   "var f = cond ? (x)=>x : x=>2" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? (x)=>x : x=>2 }"
+PASS Valid:   "var f = cond ? x=>x : x=>2" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? x=>x : x=>2 }"
+PASS Valid:   "var f = cond ? x=>x*2 : x=>2" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? x=>x*2 : x=>2 }"
+PASS Valid:   "var f = cond ? x=>x.foo : x=>2" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? x=>x.foo : x=>2 }"
+PASS Valid:   "var f = cond ? x=>x.foo : x=>x + x + x + x + x + x + x" with ReferenceError
+PASS Valid:   "function f() { var f = cond ? x=>x.foo : x=>x + x + x + x + x + x + x }"
+PASS Valid:   "var f = cond ? x=>{x.foo } : x=>x + x + x + x + x + x + (x =>x) " with ReferenceError
+PASS Valid:   "function f() { var f = cond ? x=>{x.foo } : x=>x + x + x + x + x + x + (x =>x)  }"
+PASS Valid:   "var f = (x) => x => (x) => ({y}) => y"
+PASS Valid:   "function f() { var f = (x) => x => (x) => ({y}) => y }"
+PASS Invalid: "var f = cond ? x=>x.foo; : x=>x + x + x + x + x + x + x"
+PASS Invalid: "function f() { var f = cond ? x=>x.foo; : x=>x + x + x + x + x + x + x }"
+PASS Invalid: "var f = cond ? x=>x.foo : : x=>x + x + x + x + x + x + x"
+PASS Invalid: "function f() { var f = cond ? x=>x.foo : : x=>x + x + x + x + x + x + x }"
+PASS Invalid: "var f = cond ? x=>{x.foo :} : x=>x + x + x + x + x + x + x"
+PASS Invalid: "function f() { var f = cond ? x=>{x.foo :} : x=>x + x + x + x + x + x + x }"
+PASS Invalid: "var f = cond ? x=>{x.foo } => : x=>x + x + x + x + x + x + x"
+PASS Invalid: "function f() { var f = cond ? x=>{x.foo } => : x=>x + x + x + x + x + x + x }"
 PASS e.line is 1
 PASS foo is 'PASS'
 PASS bar is 'PASS'
index 14a18a4..a58d7aa 100644 (file)
@@ -726,6 +726,20 @@ invalid("foo(([x = 25]) => x => x =>)");
 invalid("foo(([x = 25]) => x => x => {)");
 invalid("foo(x ==> x)");
 invalid("foo(x = x ==> x)");
+valid("var f = cond ? ()=>20 : ()=>20");
+valid("var f = cond ? (x)=>{x} : ()=>20");
+valid("var f = cond ? (x)=>x : ()=>{2}");
+valid("var f = cond ? (x)=>x : x=>2");
+valid("var f = cond ? x=>x : x=>2");
+valid("var f = cond ? x=>x*2 : x=>2");
+valid("var f = cond ? x=>x.foo : x=>2");
+valid("var f = cond ? x=>x.foo : x=>x + x + x + x + x + x + x");
+valid("var f = cond ? x=>{x.foo } : x=>x + x + x + x + x + x + (x =>x) ");
+valid("var f = (x) => x => (x) => ({y}) => y");
+invalid("var f = cond ? x=>x.foo; : x=>x + x + x + x + x + x + x");
+invalid("var f = cond ? x=>x.foo : : x=>x + x + x + x + x + x + x");
+invalid("var f = cond ? x=>{x.foo :} : x=>x + x + x + x + x + x + x");
+invalid("var f = cond ? x=>{x.foo } => : x=>x + x + x + x + x + x + x");
 
 
 try { eval("a.b.c = {};"); } catch(e1) { e=e1; shouldBe("e.line", "1") }
index 7851d2b..14d9def 100644 (file)
@@ -1,3 +1,45 @@
+2016-04-12  Saam barati  <sbarati@apple.com>
+
+        We incorrectly parse arrow function expressions
+        https://bugs.webkit.org/show_bug.cgi?id=156373
+
+        Reviewed by Mark Lam.
+
+        This patch removes the notion of "isEndOfArrowFunction".
+        This was a very weird function and it was incorrect.
+        It checked that the arrow functions with concise body
+        grammar production "had a valid ending". "had a valid
+        ending" is in quotes because concise body arrow functions
+        have a valid ending as long as their body has a valid
+        assignment expression. I've removed all notion of this
+        function because it was wrong and was causing us
+        to throw syntax errors on valid programs.
+
+        * parser/Lexer.cpp:
+        (JSC::Lexer<T>::nextTokenIsColon):
+        (JSC::Lexer<T>::lex):
+        (JSC::Lexer<T>::setTokenPosition): Deleted.
+        * parser/Lexer.h:
+        (JSC::Lexer::setIsReparsingFunction):
+        (JSC::Lexer::isReparsingFunction):
+        (JSC::Lexer::lineNumber):
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseInner):
+        (JSC::Parser<LexerType>::parseArrowFunctionSingleExpressionBodySourceElements):
+        (JSC::Parser<LexerType>::parseFunctionInfo):
+        * parser/Parser.h:
+        (JSC::Parser::matchIdentifierOrKeyword):
+        (JSC::Parser::tokenStart):
+        (JSC::Parser::autoSemiColon):
+        (JSC::Parser::canRecurse):
+        (JSC::Parser::isEndOfArrowFunction): Deleted.
+        (JSC::Parser::setEndOfStatement): Deleted.
+        * tests/stress/arrowfunction-others.js:
+        (testCase):
+        (simpleArrowFunction):
+        (truthy):
+        (falsey):
+
 2016-04-12  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] addStaticGlobals should emit SymbolTableEntry watchpoints to encourage constant folding in DFG
index b471d33..ec7ebcd 100644 (file)
@@ -1766,16 +1766,6 @@ bool Lexer<T>::nextTokenIsColon()
 }
 
 template <typename T>
-void Lexer<T>::setTokenPosition(JSToken* tokenRecord)
-{
-    JSTokenData* tokenData = &tokenRecord->m_data;
-    tokenData->line = lineNumber();
-    tokenData->offset = currentOffset();
-    tokenData->lineStartOffset = currentLineStartOffset();
-    ASSERT(tokenData->offset >= tokenData->lineStartOffset);
-}
-
-template <typename T>
 JSTokenType Lexer<T>::lex(JSToken* tokenRecord, unsigned lexerFlags, bool strictMode)
 {
     JSTokenData* tokenData = &tokenRecord->m_data;
index 0b5ad96..32d80fb 100644 (file)
@@ -63,7 +63,6 @@ public:
     void setIsReparsingFunction() { m_isReparsingFunction = true; }
     bool isReparsingFunction() const { return m_isReparsingFunction; }
 
-    void setTokenPosition(JSToken* tokenRecord);
     JSTokenType lex(JSToken*, unsigned, bool strictMode);
     bool nextTokenIsColon();
     int lineNumber() const { return m_lineNumber; }
index 3937ce2..a3620f0 100644 (file)
@@ -287,16 +287,7 @@ String Parser<LexerType>::parseInner(const Identifier& calleeName, SourceParseMo
         }
     }
 
-    bool validEnding;
-    if (isArrowFunctionBodyExpression) {
-        ASSERT(m_lexer->isReparsingFunction());
-        // When we reparse and stack overflow, we're not guaranteed a valid ending. If we don't run out of stack space,
-        // then of course this will always be valid because we already parsed for syntax errors. But we must
-        // be cautious in case we run out of stack space.
-        validEnding = isEndOfArrowFunction(); 
-    } else
-        validEnding = consume(EOFTOK);
-
+    bool validEnding = consume(EOFTOK);
     if (!sourceElements || !validEnding) {
         if (hasError())
             parseError = m_errorMessage;
@@ -853,13 +844,8 @@ template <class TreeBuilder> TreeSourceElements Parser<LexerType>::parseArrowFun
     
     context.setEndOffset(expr, m_lastTokenEndPosition.offset);
 
-    failIfFalse(isEndOfArrowFunction(), "Expected a ';', ']', '}', ')', ',', line terminator or EOF following a arrow function statement");
-
     JSTextPosition end = tokenEndPosition();
     
-    if (!m_lexer->prevTerminator())
-        setEndOfStatement();
-
     TreeSourceElements sourceElements = context.createSourceElements();
     TreeStatement body = context.createReturnStatement(location, expr, start, end);
     context.setEndOffset(body, m_lastTokenEndPosition.offset);
@@ -2169,9 +2155,7 @@ template <class TreeBuilder> bool Parser<LexerType>::parseFunctionInfo(TreeBuild
     
     popScope(functionScope, TreeBuilder::NeedsFreeVariableInfo);
     
-    if (functionBodyType == ArrowFunctionBodyExpression)
-        failIfFalse(isEndOfArrowFunction(), "Expected the closing ';' ',' ']' ')' '}', line terminator or EOF after arrow function");
-    else {
+    if (functionBodyType != ArrowFunctionBodyExpression) {
         matchOrFail(CLOSEBRACE, "Expected a closing '}' after a ", stringForFunctionMode(mode), " body");
         next();
     }
index 838f692..5f47681 100644 (file)
@@ -1257,11 +1257,6 @@ private:
         return isIdentifierOrKeyword(m_token);
     }
     
-    ALWAYS_INLINE bool isEndOfArrowFunction()
-    {
-        return match(SEMICOLON) || match(COMMA) || match(CLOSEPAREN) || match(CLOSEBRACE) || match(CLOSEBRACKET) || match(EOFTOK) || m_lexer->prevTerminator();
-    }
-
     ALWAYS_INLINE unsigned tokenStart()
     {
         return m_token.m_location.startOffset;
@@ -1482,11 +1477,6 @@ private:
         return allowAutomaticSemicolon();
     }
     
-    void setEndOfStatement()
-    {
-        m_lexer->setTokenPosition(&m_token);
-    }
-
     bool canRecurse()
     {
         return m_vm->isSafeToRecurse();
index 14ce0c6..59e7d9b 100644 (file)
@@ -1,17 +1,27 @@
 var testCase = function (actual, expected, message) {
-  if (actual !== expected) {
-    throw message + ". Expected '" + expected + "', but was '" + actual + "'";
-  }
+    if (actual !== expected) {
+        throw message + ". Expected '" + expected + "', but was '" + actual + "'";
+    }
 };
 
 var simpleArrowFunction = () => {};
 
 noInline(simpleArrowFunction);
 
+function truthy() { return true; }
+function falsey() { return false; }
+noInline(truthy);
+noInline(falsey);
+
 for (var i=0;i<10000;i++) {
-   testCase(Object.getPrototypeOf(simpleArrowFunction), Function.prototype, "Error: Not correct getPrototypeOf value for arrow function");
+    testCase(Object.getPrototypeOf(simpleArrowFunction), Function.prototype, "Error: Not correct getPrototypeOf value for arrow function");
+
+    testCase(simpleArrowFunction instanceof Function, true, "Error: Not correct result for instanceof method for arrow function");
 
-   testCase(simpleArrowFunction instanceof Function, true, "Error: Not correct result for instanceof method for arrow function");
+    testCase(simpleArrowFunction.constructor == Function, true, "Error: Not correct result for constructor method of arrow functio   n");
 
-   testCase(simpleArrowFunction.constructor == Function, true, "Error: Not correct result for constructor method of arrow functio   n");
+    let a1 = truthy() ? ()=>1 : ()=>2;
+    let a2 = falsey() ? ()=>2 : ()=>1;
+    testCase(a1(), 1, "Should be 1");
+    testCase(a2(), 1, "should be 1");
 }