JavaScriptCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 26 Jan 2008 09:01:40 +0000 (09:01 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 26 Jan 2008 09:01:40 +0000 (09:01 +0000)
        Reviewed by Oliver.

        - fix http://bugs.webkit.org/show_bug.cgi?id=17013
          JSC can't round trip certain for-loops

        Test: fast/js/toString-for-var-decl.html

        * kjs/nodes.h: Added PlaceholderTrueNode so we can put nodes into
        for loops without injecting the word "true" into them (nice, but not
        the bug fix). Fixed ForNode constructor so expr1WasVarDecl is set
        only when there is an expression, since it's common for the actual
        variable declaration to be moved by the parser.

        * kjs/nodes2string.cpp:
        (KJS::PlaceholderTrueNode::streamTo): Added. Empty.

LayoutTests:

        Reviewed by Oliver.

        - test for http://bugs.webkit.org/show_bug.cgi?id=17013
          JSC can't round trip certain for-loops

        * fast/js/resources/toString-for-var-decl.js: Streamlined the test a bit, with more
        of the execution within shouldBe so that exceptions are caught for us. Added a new
        test case reflecting the just-fixed bug.
        * fast/js/toString-for-var-decl-expected.txt: Updated.

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/nodes.h
JavaScriptCore/kjs/nodes2string.cpp
LayoutTests/ChangeLog
LayoutTests/fast/js/resources/toString-for-var-decl.js
LayoutTests/fast/js/toString-for-var-decl-expected.txt

index e3554fb1e832abc6b71e149abfb330edc0ac9ca1..840929bd53386329d4e8cbe29855b51f2c4f6392 100644 (file)
@@ -1,3 +1,21 @@
+2008-01-26  Darin Adler  <darin@apple.com>
+
+        Reviewed by Oliver.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=17013
+          JSC can't round trip certain for-loops
+
+        Test: fast/js/toString-for-var-decl.html
+
+        * kjs/nodes.h: Added PlaceholderTrueNode so we can put nodes into
+        for loops without injecting the word "true" into them (nice, but not
+        the bug fix). Fixed ForNode constructor so expr1WasVarDecl is set
+        only when there is an expression, since it's common for the actual
+        variable declaration to be moved by the parser.
+
+        * kjs/nodes2string.cpp:
+        (KJS::PlaceholderTrueNode::streamTo): Added. Empty.
+
 2008-01-25  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Maciej.
index 678af2dbd4874127b62029faea161c80ff68ea02..b24c22c2fa214430a1e027b47a3778166162e67b 100644 (file)
@@ -236,6 +236,13 @@ namespace KJS {
     virtual Precedence precedence() const { return PrecPrimary; }
   };
 
+  class PlaceholderTrueNode : public TrueNode {
+  public:
+    // Like TrueNode, but does not serialize as "true".
+    PlaceholderTrueNode() KJS_FAST_CALL : TrueNode() { }
+    virtual void streamTo(SourceStream&) const KJS_FAST_CALL;
+  };
+
   class NumberNode : public ExpressionNode {
   public:
     NumberNode(double v) KJS_FAST_CALL : ExpressionNode(NumberType), m_double(v) {}
@@ -1814,11 +1821,11 @@ namespace KJS {
   class ForNode : public StatementNode {
   public:
       ForNode(ExpressionNode* e1, ExpressionNode* e2, ExpressionNode* e3, StatementNode* s, bool e1WasVarDecl) KJS_FAST_CALL
-        : expr1(e1 ? e1 : new TrueNode)
-        , expr2(e2 ? e2 : new TrueNode)
-        , expr3(e3 ? e3 : new TrueNode)
+        : expr1(e1 ? e1 : new PlaceholderTrueNode)
+        , expr2(e2 ? e2 : new PlaceholderTrueNode)
+        , expr3(e3 ? e3 : new PlaceholderTrueNode)
         , statement(s)
-        , expr1WasVarDecl(e1WasVarDecl)
+        , expr1WasVarDecl(e1 && e1WasVarDecl)
     {
         ASSERT(expr1);
         ASSERT(expr2);
index 49ac1562bebb971e227c7c14796deb1328e72e4e..0432b7f501d64d7ec89a269f764dd0e386094b13 100644 (file)
@@ -287,6 +287,10 @@ void TrueNode::streamTo(SourceStream& s) const
     s << "true";
 }
 
+void PlaceholderTrueNode::streamTo(SourceStream&) const
+{
+}
+
 void NumberNode::streamTo(SourceStream& s) const
 {
     s << value();
index 32b89514700024d54d82fd3237054f67036e5407..f03af2b0794a63df83543af0e49a29a8800928ea 100644 (file)
@@ -1,3 +1,15 @@
+2008-01-26  Darin Adler  <darin@apple.com>
+
+        Reviewed by Oliver.
+
+        - test for http://bugs.webkit.org/show_bug.cgi?id=17013
+          JSC can't round trip certain for-loops
+
+        * fast/js/resources/toString-for-var-decl.js: Streamlined the test a bit, with more
+        of the execution within shouldBe so that exceptions are caught for us. Added a new
+        test case reflecting the just-fixed bug.
+        * fast/js/toString-for-var-decl-expected.txt: Updated.
+
 2008-01-25  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Maciej.
index 383602cb91fd300194d2ff23d45396af378c2d11..bbaeae95ba4095369cf73605181b8240d96993cc 100644 (file)
@@ -1,26 +1,18 @@
 description(
-"This test checks that toString() round-trip does not change meaning of functions that contain var declarations inside for -loop."
+"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 []) {}  }
 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;;) {}  }
 
-unevalf = function(x) { return '(' + x.toString() + ')'; }
+var unevalf = function(x) { return '(' + x.toString() + ')'; }
 
-try {
-    uf1 = unevalf(f1);
-    ueuf1 = unevalf(eval(unevalf(f1)));
-}  catch(e) {};
-try {
-    uf2 = unevalf(f2);
-    ueuf2 = unevalf(eval(unevalf(f2)));
-} catch(e) {};
-
-uf3 = unevalf(f3);
-
-shouldBe("ueuf1", "uf1");
-shouldBe("ueuf2", "uf2");
-shouldBe("uf2 != uf3", "true");
+shouldBe("unevalf(eval(unevalf(f1)))", "unevalf(f1)");
+shouldBe("unevalf(eval(unevalf(f2)))", "unevalf(f2)");
+shouldBe("unevalf(eval(unevalf(f3)))", "unevalf(f3)");
+shouldBe("unevalf(eval(unevalf(f4)))", "unevalf(f4)");
+shouldBe("unevalf(f2) != unevalf(f3)", "true");
 
 var successfullyParsed = true;
index 61d77031012b59bdf74a2a415a82517b64687ee5..7aee72a724aa31bf5b4ef5bc6e2a3c5e089c887b 100644 (file)
@@ -1,11 +1,13 @@
-This test checks that toString() round-trip does not change meaning of functions that contain var declarations inside for -loop.
+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.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS ueuf1 is uf1
-PASS ueuf2 is uf2
-PASS uf2 != uf3 is true
+PASS unevalf(eval(unevalf(f1))) is unevalf(f1)
+PASS unevalf(eval(unevalf(f2))) is unevalf(f2)
+PASS unevalf(eval(unevalf(f3))) is unevalf(f3)
+PASS unevalf(eval(unevalf(f4))) is unevalf(f4)
+PASS unevalf(f2) != unevalf(f3) is true
 PASS successfullyParsed is true
 
 TEST COMPLETE