Fix for bug 17012: REGRESSION: JSC can't round trip an object literal

Reviewed by Maciej.

Add logic to ensure that object literals and function expressions get
parenthesis when necessary.

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 6e8d5a7..a835b10 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,16 @@
+2008-01-25  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Maciej.
+
+        Fix for bug 17012: REGRESSION: JSC can't round trip an object literal
+
+        Add logic to ensure that object literals and function expressions get
+        parenthesis when necessary.
+
+        * kjs/nodes.h:
+        * kjs/nodes2string.cpp:
+        (KJS::SourceStream::operator<<):
+
 2008-01-24  Steve Falkenburg  <sfalken@apple.com>
 
         Build fix.

diff --git a/JavaScriptCore/kjs/nodes.h b/JavaScriptCore/kjs/nodes.h
index 2fdabd2..678af2d 100644 (file)
--- a/JavaScriptCore/kjs/nodes.h
+++ b/JavaScriptCore/kjs/nodes.h
@@ -133,7 +133,8 @@ namespace KJS {
     // Serialization.
     virtual void streamTo(SourceStream&) const KJS_FAST_CALL = 0;
     virtual Precedence precedence() const = 0;
-    
+    virtual bool needsParensIfLeftmost() const { return false; }
+
     // Used for iterative, depth-first traversal of the node tree. Does not cross function call boundaries.
     virtual void optimizeVariableAccess(SymbolTable&, DeclarationStacks::NodeStack&) KJS_FAST_CALL { }
 
@@ -435,6 +436,7 @@ namespace KJS {
     virtual JSValue* evaluate(ExecState*) KJS_FAST_CALL;
     virtual void streamTo(SourceStream&) const KJS_FAST_CALL;
     virtual Precedence precedence() const { return PrecPrimary; }
+    virtual bool needsParensIfLeftmost() const { return true; }
   private:
     RefPtr<PropertyListNode> list;
   };
@@ -2020,6 +2022,7 @@ namespace KJS {
     virtual JSValue *evaluate(ExecState*) KJS_FAST_CALL;
     virtual void streamTo(SourceStream&) const KJS_FAST_CALL;
     virtual Precedence precedence() const { return PrecMember; }
+    virtual bool needsParensIfLeftmost() const { return true; }
   private:
     void addParams() KJS_FAST_CALL;
     // Used for streamTo

diff --git a/JavaScriptCore/kjs/nodes2string.cpp b/JavaScriptCore/kjs/nodes2string.cpp
index 577ea69..49ac156 100644 (file)
--- a/JavaScriptCore/kjs/nodes2string.cpp
+++ b/JavaScriptCore/kjs/nodes2string.cpp
@@ -41,7 +41,7 @@ enum DotExprType { DotExpr };
 
 class SourceStream {
 public:
-    SourceStream() : m_numberNeedsParens(false), m_precedence(PrecExpression) { }
+    SourceStream() : m_numberNeedsParens(false), m_atStartOfStatement(true), m_precedence(PrecExpression) { }
     UString toString() const { return m_string; }
     SourceStream& operator<<(const Identifier&);
     SourceStream& operator<<(const UString&);
@@ -60,6 +60,7 @@ private:
     UString m_string;
     UString m_spacesForIndentation;
     bool m_numberNeedsParens;
+    bool m_atStartOfStatement;
     Precedence m_precedence;
 };
 
@@ -150,6 +151,7 @@ static bool isParserRoundTripNumber(const UString& string)
 SourceStream& SourceStream::operator<<(char c)
 {
     m_numberNeedsParens = false;
+    m_atStartOfStatement = false;
     UChar ch(c);
     m_string.append(ch);
     return *this;
@@ -158,6 +160,7 @@ SourceStream& SourceStream::operator<<(char c)
 SourceStream& SourceStream::operator<<(const char* s)
 {
     m_numberNeedsParens = false;
+    m_atStartOfStatement = false;
     m_string += s;
     return *this;
 }
@@ -166,6 +169,7 @@ SourceStream& SourceStream::operator<<(double value)
 {
     bool needParens = m_numberNeedsParens;
     m_numberNeedsParens = false;
+    m_atStartOfStatement = false;
 
     if (needParens)
         m_string.append('(');
@@ -179,6 +183,7 @@ SourceStream& SourceStream::operator<<(double value)
 SourceStream& SourceStream::operator<<(const UString& s)
 {
     m_numberNeedsParens = false;
+    m_atStartOfStatement = false;
     m_string += s;
     return *this;
 }
@@ -186,13 +191,14 @@ SourceStream& SourceStream::operator<<(const UString& s)
 SourceStream& SourceStream::operator<<(const Identifier& s)
 {
     m_numberNeedsParens = false;
+    m_atStartOfStatement = false;
     m_string += s.ustring();
     return *this;
 }
 
 SourceStream& SourceStream::operator<<(const Node* n)
 {
-    bool needParens = m_precedence != PrecExpression && n->precedence() > m_precedence;
+    bool needParens = (m_precedence != PrecExpression && n->precedence() > m_precedence) || (m_atStartOfStatement && n->needsParensIfLeftmost());
     m_precedence = PrecExpression;
     if (n) {
         if (needParens)
@@ -207,6 +213,7 @@ SourceStream& SourceStream::operator<<(const Node* n)
 SourceStream& SourceStream::operator<<(EndlType)
 {
     m_numberNeedsParens = false;
+    m_atStartOfStatement = true;
     m_string.append('\n');
     m_string.append(m_spacesForIndentation);
     return *this;
@@ -215,6 +222,7 @@ SourceStream& SourceStream::operator<<(EndlType)
 SourceStream& SourceStream::operator<<(IndentType)
 {
     m_numberNeedsParens = false;
+    m_atStartOfStatement = false;
     m_spacesForIndentation += "  ";
     return *this;
 }
@@ -222,6 +230,7 @@ SourceStream& SourceStream::operator<<(IndentType)
 SourceStream& SourceStream::operator<<(UnindentType)
 {
     m_numberNeedsParens = false;
+    m_atStartOfStatement = false;
     m_spacesForIndentation = m_spacesForIndentation.substr(0, m_spacesForIndentation.size() - 2);
     return *this;
 }

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6e28790..32b8951 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2008-01-25  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Maciej.
+
+        Add additional check for using Function.toString on a function with object literals
+
+        * fast/js/function-toString-parentheses-expected.txt:
+        * fast/js/resources/function-toString-parentheses.js:
+
 2008-01-25  Darin Adler  <darin@apple.com>
 
         Reviewed by Anders.

diff --git a/LayoutTests/fast/js/function-toString-parentheses-expected.txt b/LayoutTests/fast/js/function-toString-parentheses-expected.txt
index d697f53..d5f1f3f 100644 (file)
--- a/LayoutTests/fast/js/function-toString-parentheses-expected.txt
+++ b/LayoutTests/fast/js/function-toString-parentheses-expected.txt
@@ -402,6 +402,10 @@ PASS compileAndSerialize('(!a)++') is '(!a)++'
 PASS compileAndSerialize('!a--') is '!a--'
 PASS compileAndSerialize('!(a--)') is '!a--'
 PASS compileAndSerialize('(!a)--') is '(!a)--'
+PASS compileAndSerializeLeftmostTest('({ }).x') is '({ }).x'
+PASS compileAndSerializeLeftmostTest('x = { }') is 'x = { }'
+PASS compileAndSerializeLeftmostTest('(function () { })()') is '(function () { })()'
+PASS compileAndSerializeLeftmostTest('x = function () { }') is 'x = function () { }'
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/LayoutTests/fast/js/resources/function-toString-parentheses.js b/LayoutTests/fast/js/resources/function-toString-parentheses.js
index e01c2fe..3aebfeb 100644 (file)
--- a/LayoutTests/fast/js/resources/function-toString-parentheses.js
+++ b/LayoutTests/fast/js/resources/function-toString-parentheses.js
@@ -13,6 +13,16 @@ function compileAndSerialize(expression)
     return serializedString;
 }
 
+function compileAndSerializeLeftmostTest(expression)
+{
+    var f = eval("(function () { " + expression + "; })");
+    var serializedString = f.toString();
+    serializedString = serializedString.replace(/[ \t\r\n]+/g, " ");
+    serializedString = serializedString.replace("function () { ", "");
+    serializedString = serializedString.replace("; }", "");
+    return serializedString;
+}
+
 var removesExtraParentheses = compileAndSerialize("(a + b) + c") == "a + b + c";
 
 function testLeftAssociativeSame(opA, opB)
@@ -127,6 +137,9 @@ shouldBe("compileAndSerialize('(!a)++')", "'(!a)++'");
 shouldBe("compileAndSerialize('!a--')", "'!a--'");
 shouldBe("compileAndSerialize('!(a--)')", "'!a--'");
 shouldBe("compileAndSerialize('(!a)--')", "'(!a)--'");
-
+shouldBe("compileAndSerializeLeftmostTest('({ }).x')", "'({ }).x'");
+shouldBe("compileAndSerializeLeftmostTest('x = { }')", "'x = { }'");
+shouldBe("compileAndSerializeLeftmostTest('(function () { })()')", "'(function () { })()'");
+shouldBe("compileAndSerializeLeftmostTest('x = function () { }')", "'x = function () { }'");
 
 var successfullyParsed = true;