Try to kill initialiser expression in for-in statements
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Sep 2013 21:19:38 +0000 (21:19 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Sep 2013 21:19:38 +0000 (21:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=121311

Reviewed by Gavin Barraclough.

Source/JavaScriptCore:

We'd like to get rid of this pointless initialiser expression
in for-in statements.  Unfortunately we have to keep the no_in
variant of expression parsing to avoid ambiguity in the grammar.
There's a possibility that this will need to be rolled out, but
we'll need to live on it to see.

* bytecompiler/NodesCodegen.cpp:
(JSC::ForInNode::emitBytecode):
* parser/ASTBuilder.h:
(JSC::ASTBuilder::createForInLoop):
* parser/NodeConstructors.h:
(JSC::ForInNode::ForInNode):
* parser/Nodes.h:
* parser/Parser.cpp:
(JSC::::parseForStatement):
* parser/SyntaxChecker.h:
(JSC::SyntaxChecker::createForInLoop):

LayoutTests:

Update test cases to represent the new reality

* js/line-column-numbers-expected.txt:
* js/line-column-numbers.html:
* js/parser-syntax-check-expected.txt:
* js/script-tests/function-declaration-statement.js:
* js/script-tests/line-column-numbers.js:
(try.toFuzz22b):
* js/script-tests/parser-syntax-check.js:
* js/script-tests/toString-for-var-decl.js:
(f1):

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/js/line-column-numbers-expected.txt
LayoutTests/js/line-column-numbers.html
LayoutTests/js/parser-syntax-check-expected.txt
LayoutTests/js/script-tests/function-declaration-statement.js
LayoutTests/js/script-tests/line-column-numbers.js
LayoutTests/js/script-tests/parser-syntax-check.js
LayoutTests/js/script-tests/toString-for-var-decl.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
Source/JavaScriptCore/parser/ASTBuilder.h
Source/JavaScriptCore/parser/NodeConstructors.h
Source/JavaScriptCore/parser/Nodes.h
Source/JavaScriptCore/parser/Parser.cpp
Source/JavaScriptCore/parser/SyntaxChecker.h

index 742d9cc..10ae2a6 100644 (file)
@@ -1,3 +1,22 @@
+2013-09-13  Oliver Hunt  <oliver@apple.com>
+
+        Try to kill initialiser expression in for-in statements
+        https://bugs.webkit.org/show_bug.cgi?id=121311
+
+        Reviewed by Gavin Barraclough.
+
+        Update test cases to represent the new reality
+
+        * js/line-column-numbers-expected.txt:
+        * js/line-column-numbers.html:
+        * js/parser-syntax-check-expected.txt:
+        * js/script-tests/function-declaration-statement.js:
+        * js/script-tests/line-column-numbers.js:
+        (try.toFuzz22b):
+        * js/script-tests/parser-syntax-check.js:
+        * js/script-tests/toString-for-var-decl.js:
+        (f1):
+
 2013-09-13  Alexey Proskuryakov  <ap@apple.com>
 
         Flaky Test: http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
index 82e74be..7fbd84e 100644 (file)
@@ -142,7 +142,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
     1   global code at line-column-numbers.html:209:13
 
 --> Case 22 Stack Trace:
-    0   toFuzz22 at line-column-numbers.html:220:41
+    0   toFuzz22 at line-column-numbers.html:220:36
     1   global code at line-column-numbers.html:224:13
 
 --> Case 1 Stack Trace:
@@ -284,7 +284,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
     1   global code at line-column-numbers.js:144:14
 
 --> Case 22 Stack Trace:
-    0   toFuzz22b at line-column-numbers.js:153:41
+    0   toFuzz22b at line-column-numbers.js:153:36
     1   global code at line-column-numbers.js:157:14
 
 PASS successfullyParsed is true
index c237b1a..58f703b 100644 (file)
@@ -217,8 +217,8 @@ try {
 <script>
 try {
     function toFuzz22() {
-        for (var conf = new ConfigObject({
-        }) in str1.localeCompare) {
+        var conf = new ConfigObject({})
+        for (conf in str1.localeCompare) {
         }
     }
     toFuzz22();
index 856711e..526821a 100644 (file)
@@ -431,8 +431,8 @@ PASS Valid:   "for ((a ? b : c) in c) break"
 PASS Valid:   "function f() { for ((a ? b : c) in c) break }"
 PASS Valid:   "for (var a in b in c) break"
 PASS Valid:   "function f() { for (var a in b in c) break }"
-PASS Valid:   "for (var a = 5 += 6 in b) break"
-PASS Valid:   "function f() { for (var a = 5 += 6 in b) break }"
+PASS Invalid: "for (var a = 5 += 6 in b) break"
+PASS Invalid: "function f() { for (var a = 5 += 6 in b) break }"
 PASS Invalid: "for (var a += 5 in b) break"
 PASS Invalid: "function f() { for (var a += 5 in b) break }"
 PASS Invalid: "for (var a = in b) break"
@@ -443,8 +443,8 @@ PASS Invalid: "for (var a = -6, b in b) break"
 PASS Invalid: "function f() { for (var a = -6, b in b) break }"
 PASS Invalid: "for (var a, b = 8 in b) break"
 PASS Invalid: "function f() { for (var a, b = 8 in b) break }"
-PASS Valid:   "for (var a = (b in c) in d) break"
-PASS Valid:   "function f() { for (var a = (b in c) in d) break }"
+PASS Invalid: "for (var a = (b in c) in d) break"
+PASS Invalid: "function f() { for (var a = (b in c) in d) break }"
 PASS Invalid: "for (var a = (b in c in d) break"
 PASS Invalid: "function f() { for (var a = (b in c in d) break }"
 PASS Invalid: "for (var (a) in b) { }"
index ab23cfa..02fee15 100644 (file)
@@ -122,7 +122,7 @@ shouldBeTrue("forInVarTest()");
 function forInVarInitTest()
 {
     var a;
-    for (var a = false in { field: false })
+    for (var a in { field: false })
         function f()
         {
             return true;
index a117c82..88119ed 100644 (file)
@@ -150,8 +150,8 @@ try {
 testId++;
 try {
     function toFuzz22b() {
-        for (var conf = new ConfigObject({
-        }) in str1.localeCompare) {
+        var conf = new ConfigObject({})
+        for (conf in str1.localeCompare) {
         }
     }
     toFuzz22b();
index 5251a03..a7bf763 100644 (file)
@@ -284,13 +284,13 @@ valid  ("for ((a, b) in c) break");
 invalid("for (a ? b : c in c) break");
 valid  ("for ((a ? b : c) in c) break");
 valid  ("for (var a in b in c) break");
-valid  ("for (var a = 5 += 6 in b) break");
+invalid("for (var a = 5 += 6 in b) break");
 invalid("for (var a += 5 in b) break");
 invalid("for (var a = in b) break");
 invalid("for (var a, b in b) break");
 invalid("for (var a = -6, b in b) break");
 invalid("for (var a, b = 8 in b) break");
-valid  ("for (var a = (b in c) in d) break");
+invalid("for (var a = (b in c) in d) break");
 invalid("for (var a = (b in c in d) break");
 invalid("for (var (a) in b) { }");
 valid  ("for (var a = 7, b = c < d >= d ; f()[6]++ ; --i()[1]++ ) {}");
index b41bcb4..3c1cde6 100644 (file)
@@ -2,7 +2,7 @@ description(
 "This test checks for a couple of specific ways that bugs in toString() round trips have changed the meanings of functions with var declarations inside for loops."
 );
 
-function f1() { for (var j = 1 in []) {}  }
+function f1() { for (var j in []) {}  }
 var f2 = function () { for (var j = 1; j < 10; ++j) {}  }
 var f3 = function () { for (j = 1;j < 10; ++j) {}  }
 var f4 = function () { for (var j;;) {}  }
index 8105bbf..2ff89b8 100644 (file)
@@ -1,3 +1,28 @@
+2013-09-13  Oliver Hunt  <oliver@apple.com>
+
+        Try to kill initialiser expression in for-in statements
+        https://bugs.webkit.org/show_bug.cgi?id=121311
+
+        Reviewed by Gavin Barraclough.
+
+        We'd like to get rid of this pointless initialiser expression
+        in for-in statements.  Unfortunately we have to keep the no_in
+        variant of expression parsing to avoid ambiguity in the grammar.
+        There's a possibility that this will need to be rolled out, but
+        we'll need to live on it to see.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::ForInNode::emitBytecode):
+        * parser/ASTBuilder.h:
+        (JSC::ASTBuilder::createForInLoop):
+        * parser/NodeConstructors.h:
+        (JSC::ForInNode::ForInNode):
+        * parser/Nodes.h:
+        * parser/Parser.cpp:
+        (JSC::::parseForStatement):
+        * parser/SyntaxChecker.h:
+        (JSC::SyntaxChecker::createForInLoop):
+
 2013-09-12  Michael Saboff  <msaboff@apple.com>
 
         fourthTier: Change JSStack to grow from high to low addresses
index 11de68e..e0c6a1a 100644 (file)
@@ -1673,9 +1673,6 @@ void ForInNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 
     generator.emitDebugHook(WillExecuteStatement, firstLine(), lastLine(), startOffset(), lineStartOffset());
 
-    if (m_init)
-        generator.emitNode(generator.ignoredResult(), m_init);
-
     RefPtr<RegisterID> base = generator.newTemporary();
     generator.emitNode(base.get(), m_expr);
     RefPtr<RegisterID> i = generator.newTemporary();
index 4281e7c..95c98d0 100644 (file)
@@ -361,9 +361,9 @@ public:
         return result;
     }
 
-    StatementNode* createForInLoop(const JSTokenLocation& location, const Identifier* ident, ExpressionNode* initializer, ExpressionNode* iter, StatementNode* statements, const JSTextPosition& start, const JSTextPosition& divot, const JSTextPosition& end, const JSTextPosition& initStart, const JSTextPosition& initEnd, int startLine, int endLine)
+    StatementNode* createForInLoop(const JSTokenLocation& location, const Identifier* ident, ExpressionNode* iter, StatementNode* statements, const JSTextPosition& start, const JSTextPosition& divot, const JSTextPosition& end, int startLine, int endLine)
     {
-        ForInNode* result = new (m_vm) ForInNode(m_vm, location, *ident, initializer, iter, statements, initStart, start, initEnd);
+        ForInNode* result = new (m_vm) ForInNode(m_vm, location, *ident, iter, statements, start);
         result->setLoc(startLine, endLine, location.startOffset, location.lineStartOffset);
         setExceptionLocation(result, start, divot + 1, end);
         return result;
index b31c014..5ccc779 100644 (file)
@@ -808,27 +808,18 @@ inline ResolveNode::ResolveNode(const JSTokenLocation& location, const Identifie
 
     inline ForInNode::ForInNode(const JSTokenLocation& location, ExpressionNode* l, ExpressionNode* expr, StatementNode* statement)
         : StatementNode(location)
-        , m_init(0)
         , m_lexpr(l)
         , m_expr(expr)
         , m_statement(statement)
     {
     }
 
-    inline ForInNode::ForInNode(VM* vm, const JSTokenLocation& location, const Identifier& ident, ExpressionNode* in, ExpressionNode* expr, StatementNode* statement, const JSTextPosition& divot, const JSTextPosition& divotStart, const JSTextPosition& divotEnd)
+    inline ForInNode::ForInNode(VM* vm, const JSTokenLocation& location, const Identifier& ident, ExpressionNode* expr, StatementNode* statement, const JSTextPosition& divotStart)
         : StatementNode(location)
-        , m_init(0)
         , m_lexpr(new (vm) ResolveNode(location, ident, divotStart))
         , m_expr(expr)
         , m_statement(statement)
     {
-        if (in) {
-            AssignResolveNode* node = new (vm) AssignResolveNode(location, ident, in);
-            ASSERT(divot.offset >= divot.lineStartOffset);
-            node->setExceptionSourceCode(divot, divotStart, divotEnd);
-            m_init = node;
-        }
-        // for( var foo = bar in baz )
     }
 
 } // namespace JSC
index 4b6b8da..5bcacab 100644 (file)
@@ -1242,12 +1242,11 @@ namespace JSC {
     class ForInNode : public StatementNode, public ThrowableExpressionData {
     public:
         ForInNode(const JSTokenLocation&, ExpressionNode*, ExpressionNode*, StatementNode*);
-        ForInNode(VM*, const JSTokenLocation&, const Identifier&, ExpressionNode*, ExpressionNode*, StatementNode*, const JSTextPosition& divot, const JSTextPosition& divotStart, const JSTextPosition& divotEnd);
+        ForInNode(VM*, const JSTokenLocation&, const Identifier&, ExpressionNode*, StatementNode*, const JSTextPosition& divotStart);
 
     private:
         virtual void emitBytecode(BytecodeGenerator&, RegisterID* = 0);
 
-        ExpressionNode* m_init;
         ExpressionNode* m_lexpr;
         ExpressionNode* m_expr;
         StatementNode* m_statement;
index 36eb735..97f38d8 100644 (file)
@@ -368,7 +368,8 @@ template <class TreeBuilder> TreeStatement Parser<LexerType>::parseForStatement(
             goto standardForLoop;
         
         failIfFalse(declarations == 1);
-        
+        failIfTrueWithMessage(forInInitializer, "Cannot use initialiser syntax in a for-in loop");
+
         // Handle for-in with var declaration
         JSTextPosition inLocation = tokenStartPosition();
         consumeOrFail(INTOKEN);
@@ -386,7 +387,7 @@ template <class TreeBuilder> TreeStatement Parser<LexerType>::parseForStatement(
         endLoop();
         failIfFalse(statement);
         
-        return context.createForInLoop(location, forInTarget, forInInitializer, expr, statement, declsStart, inLocation, exprEnd, initStart, initEnd, startLine, endLine);
+        return context.createForInLoop(location, forInTarget, expr, statement, declsStart, inLocation, exprEnd, startLine, endLine);
     }
     
     if (!match(SEMICOLON)) {
index 6e600eb..9929caf 100644 (file)
@@ -185,7 +185,7 @@ public:
     int createIfStatement(const JSTokenLocation&, int, int, int, int) { return 1; }
     int createIfStatement(const JSTokenLocation&, int, int, int, int, int) { return 1; }
     int createForLoop(const JSTokenLocation&, int, int, int, int, int, int) { return 1; }
-    int createForInLoop(const JSTokenLocation&, const Identifier*, int, int, int, int, int, int, int, int, int, int) { return 1; }
+    int createForInLoop(const JSTokenLocation&, const Identifier*, int, int, int, int, int, int, int) { return 1; }
     int createForInLoop(const JSTokenLocation&, int, int, int, int, int, int, int, int) { return 1; }
     int createEmptyStatement(const JSTokenLocation&) { return 1; }
     int createVarStatement(const JSTokenLocation&, int, int, int) { return 1; }