JavaScriptCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 27 Jan 2008 04:21:43 +0000 (04:21 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 27 Jan 2008 04:21:43 +0000 (04:21 +0000)
        Reviewed by Oliver.

        - fix http://bugs.webkit.org/show_bug.cgi?id=17027
          Incorrect Function.toString behaviour with read/modify/write operators performed on negative numbers

        Test: fast/js/function-toString-parentheses.html

        The problem here was that a NumberNode with a negative number in it had the wrong
        precedence. It's not a primary expression, it's a unary operator with a primary
        expression after it.

        Once the precedence of NumberNode was fixed, the cases from bug 17020 were also
        fixed without trying to treat bracket nodes like dot nodes. That wasn't needed.
        The reason we handle numbers before dot nodes specially is that the dot is a
        legal character in a number. The same is not true of a bracket. Eventually we
        could get smarter, and only add the parentheses when there is actual ambiguity.
        There is none if the string form of the number already has a dot in it, or if
        it's a number with a alphabetic name like infinity or NAN.

        * kjs/nodes.h: Renamed back from ObjectAccess to DotExpr.
        (KJS::NumberNode::precedence): Return PrecUnary for negative numbers, since
        they serialize as a unary operator, not a primary expression.
        * kjs/nodes2string.cpp:
        (KJS::SourceStream::operator<<): Clear m_numberNeedsParens if this adds
        parens; one set is enough.
        (KJS::bracketNodeStreamTo): Remove unneeded special flag here. Normal
        operator precedence suffices.
        (KJS::NewExprNode::streamTo): Ditto.

LayoutTests:

        Reviewed by Oliver.

        - test for http://bugs.webkit.org/show_bug.cgi?id=17027
          Incorrect Function.toString behaviour with read/modify/write operators performed on negative numbers

        * fast/js/function-toString-parentheses-expected.txt: Updated.
        * fast/js/resources/function-toString-parentheses.js: More test cases.

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/nodes.h
JavaScriptCore/kjs/nodes2string.cpp
LayoutTests/ChangeLog
LayoutTests/fast/js/function-toString-parentheses-expected.txt
LayoutTests/fast/js/resources/function-toString-parentheses.js

index b5e2a01..a37be24 100644 (file)
@@ -1,3 +1,34 @@
+2008-01-26  Darin Adler  <darin@apple.com>
+
+        Reviewed by Oliver.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=17027
+          Incorrect Function.toString behaviour with read/modify/write operators performed on negative numbers
+
+        Test: fast/js/function-toString-parentheses.html
+
+        The problem here was that a NumberNode with a negative number in it had the wrong
+        precedence. It's not a primary expression, it's a unary operator with a primary
+        expression after it.
+
+        Once the precedence of NumberNode was fixed, the cases from bug 17020 were also
+        fixed without trying to treat bracket nodes like dot nodes. That wasn't needed.
+        The reason we handle numbers before dot nodes specially is that the dot is a
+        legal character in a number. The same is not true of a bracket. Eventually we
+        could get smarter, and only add the parentheses when there is actual ambiguity.
+        There is none if the string form of the number already has a dot in it, or if
+        it's a number with a alphabetic name like infinity or NAN.
+
+        * kjs/nodes.h: Renamed back from ObjectAccess to DotExpr.
+        (KJS::NumberNode::precedence): Return PrecUnary for negative numbers, since
+        they serialize as a unary operator, not a primary expression.
+        * kjs/nodes2string.cpp:
+        (KJS::SourceStream::operator<<): Clear m_numberNeedsParens if this adds
+        parens; one set is enough.
+        (KJS::bracketNodeStreamTo): Remove unneeded special flag here. Normal
+        operator precedence suffices.
+        (KJS::NewExprNode::streamTo): Ditto.
+
 2008-01-26  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Maciej and Darin.
index bc06711..9467234 100644 (file)
@@ -30,6 +30,7 @@
 #include "regexp.h"
 #include "SymbolTable.h"
 #include <wtf/ListRefPtr.h>
+#include <wtf/MathExtras.h>
 #include <wtf/OwnPtr.h>
 #include <wtf/Vector.h>
 
@@ -252,7 +253,7 @@ namespace KJS {
     virtual int32_t evaluateToInt32(ExecState*) KJS_FAST_CALL;
     virtual uint32_t evaluateToUInt32(ExecState*) KJS_FAST_CALL;
     virtual void streamTo(SourceStream&) const KJS_FAST_CALL;
-    virtual Precedence precedence() const { return PrecPrimary; }
+    virtual Precedence precedence() const { return signbit(m_double) ? PrecUnary : PrecPrimary; }
 
     virtual bool isNumber() const KJS_FAST_CALL { return true; }
     double value() const KJS_FAST_CALL { return m_double; }
index 222ccdb..50a9b06 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *  Copyright (C) 2002 Harri Porten (porten@kde.org)
- *  Copyright (C) 2003, 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
+ *  Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *  Copyright (C) 2007 Eric Seidel <eric@webkit.org>
  *
  *  This library is free software; you can redistribute it and/or
@@ -37,7 +37,7 @@ namespace KJS {
 enum EndlType { Endl };
 enum IndentType { Indent };
 enum UnindentType { Unindent };
-enum ObjectAccessType { ObjectAccess };
+enum DotExprType { DotExpr };
 
 class SourceStream {
 public:
@@ -51,7 +51,7 @@ public:
     SourceStream& operator<<(EndlType);
     SourceStream& operator<<(IndentType);
     SourceStream& operator<<(UnindentType);
-    SourceStream& operator<<(ObjectAccessType);
+    SourceStream& operator<<(DotExprType);
     SourceStream& operator<<(Precedence);
     SourceStream& operator<<(const Node*);
     template <typename T> SourceStream& operator<<(const RefPtr<T>& n) { return *this << n.get(); }
@@ -200,13 +200,15 @@ SourceStream& SourceStream::operator<<(const Node* n)
 {
     bool needParens = (m_precedence != PrecExpression && n->precedence() > m_precedence) || (m_atStartOfStatement && n->needsParensIfLeftmost());
     m_precedence = PrecExpression;
-    if (n) {
-        if (needParens)
-            m_string.append('(');
-        n->streamTo(*this);
-        if (needParens)
-            m_string.append(')');
+    if (!n)
+        return *this;
+    if (needParens) {
+        m_numberNeedsParens = false;
+        m_string.append('(');
     }
+    n->streamTo(*this);
+    if (needParens)
+        m_string.append(')');
     return *this;
 }
 
@@ -235,7 +237,7 @@ SourceStream& SourceStream::operator<<(UnindentType)
     return *this;
 }
 
-inline SourceStream& SourceStream::operator<<(ObjectAccessType)
+inline SourceStream& SourceStream::operator<<(DotExprType)
 {
     m_numberNeedsParens = true;
     return *this;
@@ -263,12 +265,12 @@ template <typename T> static inline void streamLeftAssociativeBinaryOperator(Sou
 
 static inline void bracketNodeStreamTo(SourceStream& s, const RefPtr<ExpressionNode>& base, const RefPtr<ExpressionNode>& subscript)
 {
-    s << ObjectAccess << PrecCall << base.get() << "[" << subscript.get() << "]";
+    s << PrecCall << base.get() << "[" << subscript.get() << "]";
 }
 
 static inline void dotNodeStreamTo(SourceStream& s, const RefPtr<ExpressionNode>& base, const Identifier& ident)
 {
-    s << ObjectAccess << PrecCall << base.get() << "." << ident;
+    s << DotExpr << PrecCall << base.get() << "." << ident;
 }
 
 // --------
@@ -414,7 +416,7 @@ void ArgumentsNode::streamTo(SourceStream& s) const
 
 void NewExprNode::streamTo(SourceStream& s) const
 {
-    s << "new " << ObjectAccess << PrecMember << expr << args;
+    s << "new " << PrecMember << expr << args;
 }
 
 void FunctionCallValueNode::streamTo(SourceStream& s) const
index 8cc0681..5998fea 100644 (file)
@@ -1,3 +1,13 @@
+2008-01-26  Darin Adler  <darin@apple.com>
+
+        Reviewed by Oliver.
+
+        - test for http://bugs.webkit.org/show_bug.cgi?id=17027
+          Incorrect Function.toString behaviour with read/modify/write operators performed on negative numbers
+
+        * fast/js/function-toString-parentheses-expected.txt: Updated.
+        * fast/js/resources/function-toString-parentheses.js: More test cases.
+
 2008-01-26  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Maciej and Darin.
index 70a2b99..3b1317c 100644 (file)
@@ -408,6 +408,75 @@ PASS compileAndSerialize('(-1)[a] += b') is '(-1)[a] += b'
 PASS compileAndSerialize('(-1)[a]++') is '(-1)[a]++'
 PASS compileAndSerialize('++(-1)[a]') is '++(-1)[a]'
 PASS compileAndSerialize('(-1)[a]()') is '(-1)[a]()'
+PASS compileAndSerialize('new (-1)()') is 'new (-1)()'
+PASS compileAndSerialize('(-1).a') is '(-1).a'
+PASS compileAndSerialize('(-1).a = b') is '(-1).a = b'
+PASS compileAndSerialize('(-1).a += b') is '(-1).a += b'
+PASS compileAndSerialize('(-1).a++') is '(-1).a++'
+PASS compileAndSerialize('++(-1).a') is '++(-1).a'
+PASS compileAndSerialize('(-1).a()') is '(-1).a()'
+PASS compileAndSerialize('(- 0)[a]') is '(- 0)[a]'
+PASS compileAndSerialize('(- 0)[a] = b') is '(- 0)[a] = b'
+PASS compileAndSerialize('(- 0)[a] += b') is '(- 0)[a] += b'
+PASS compileAndSerialize('(- 0)[a]++') is '(- 0)[a]++'
+PASS compileAndSerialize('++(- 0)[a]') is '++(- 0)[a]'
+PASS compileAndSerialize('(- 0)[a]()') is '(- 0)[a]()'
+PASS compileAndSerialize('new (- 0)()') is 'new (- 0)()'
+PASS compileAndSerialize('(- 0).a') is '(- 0).a'
+PASS compileAndSerialize('(- 0).a = b') is '(- 0).a = b'
+PASS compileAndSerialize('(- 0).a += b') is '(- 0).a += b'
+PASS compileAndSerialize('(- 0).a++') is '(- 0).a++'
+PASS compileAndSerialize('++(- 0).a') is '++(- 0).a'
+PASS compileAndSerialize('(- 0).a()') is '(- 0).a()'
+PASS compileAndSerialize('(1)[a]') is '1[a]'
+PASS compileAndSerialize('(1)[a] = b') is '1[a] = b'
+PASS compileAndSerialize('(1)[a] += b') is '1[a] += b'
+PASS compileAndSerialize('(1)[a]++') is '1[a]++'
+PASS compileAndSerialize('++(1)[a]') is '++1[a]'
+PASS compileAndSerialize('(1)[a]()') is '1[a]()'
+PASS compileAndSerialize('new (1)()') is 'new 1()'
+PASS compileAndSerialize('(1).a') is '(1).a'
+PASS compileAndSerialize('(1).a = b') is '(1).a = b'
+PASS compileAndSerialize('(1).a += b') is '(1).a += b'
+PASS compileAndSerialize('(1).a++') is '(1).a++'
+PASS compileAndSerialize('++(1).a') is '++(1).a'
+PASS compileAndSerialize('(1).a()') is '(1).a()'
+PASS compileAndSerialize('(-1) = a') is '(-1) = a'
+PASS compileAndSerialize('(- 0) = a') is '(- 0) = a'
+PASS compileAndSerialize('1 = a') is '1 = a'
+PASS compileAndSerialize('(-1) *= a') is '(-1) *= a'
+PASS compileAndSerialize('(- 0) *= a') is '(- 0) *= a'
+PASS compileAndSerialize('1 *= a') is '1 *= a'
+PASS compileAndSerialize('(-1) /= a') is '(-1) /= a'
+PASS compileAndSerialize('(- 0) /= a') is '(- 0) /= a'
+PASS compileAndSerialize('1 /= a') is '1 /= a'
+PASS compileAndSerialize('(-1) %= a') is '(-1) %= a'
+PASS compileAndSerialize('(- 0) %= a') is '(- 0) %= a'
+PASS compileAndSerialize('1 %= a') is '1 %= a'
+PASS compileAndSerialize('(-1) += a') is '(-1) += a'
+PASS compileAndSerialize('(- 0) += a') is '(- 0) += a'
+PASS compileAndSerialize('1 += a') is '1 += a'
+PASS compileAndSerialize('(-1) -= a') is '(-1) -= a'
+PASS compileAndSerialize('(- 0) -= a') is '(- 0) -= a'
+PASS compileAndSerialize('1 -= a') is '1 -= a'
+PASS compileAndSerialize('(-1) <<= a') is '(-1) <<= a'
+PASS compileAndSerialize('(- 0) <<= a') is '(- 0) <<= a'
+PASS compileAndSerialize('1 <<= a') is '1 <<= a'
+PASS compileAndSerialize('(-1) >>= a') is '(-1) >>= a'
+PASS compileAndSerialize('(- 0) >>= a') is '(- 0) >>= a'
+PASS compileAndSerialize('1 >>= a') is '1 >>= a'
+PASS compileAndSerialize('(-1) >>>= a') is '(-1) >>>= a'
+PASS compileAndSerialize('(- 0) >>>= a') is '(- 0) >>>= a'
+PASS compileAndSerialize('1 >>>= a') is '1 >>>= a'
+PASS compileAndSerialize('(-1) &= a') is '(-1) &= a'
+PASS compileAndSerialize('(- 0) &= a') is '(- 0) &= a'
+PASS compileAndSerialize('1 &= a') is '1 &= a'
+PASS compileAndSerialize('(-1) ^= a') is '(-1) ^= a'
+PASS compileAndSerialize('(- 0) ^= a') is '(- 0) ^= a'
+PASS compileAndSerialize('1 ^= a') is '1 ^= a'
+PASS compileAndSerialize('(-1) |= a') is '(-1) |= a'
+PASS compileAndSerialize('(- 0) |= a') is '(- 0) |= a'
+PASS compileAndSerialize('1 |= a') is '1 |= a'
 PASS compileAndSerializeLeftmostTest('({ }).x') is '({ }).x'
 PASS compileAndSerializeLeftmostTest('x = { }') is 'x = { }'
 PASS compileAndSerializeLeftmostTest('(function () { })()') is '(function () { })()'
index 41a0c12..2491761 100644 (file)
@@ -137,13 +137,62 @@ shouldBe("compileAndSerialize('(!a)++')", "'(!a)++'");
 shouldBe("compileAndSerialize('!a--')", "'!a--'");
 shouldBe("compileAndSerialize('!(a--)')", "'!a--'");
 shouldBe("compileAndSerialize('(!a)--')", "'(!a)--'");
+
 shouldBe("compileAndSerialize('(-1)[a]')", "'(-1)[a]'");
 shouldBe("compileAndSerialize('(-1)[a] = b')", "'(-1)[a] = b'");
 shouldBe("compileAndSerialize('(-1)[a] += b')", "'(-1)[a] += b'");
 shouldBe("compileAndSerialize('(-1)[a]++')", "'(-1)[a]++'");
 shouldBe("compileAndSerialize('++(-1)[a]')", "'++(-1)[a]'");
 shouldBe("compileAndSerialize('(-1)[a]()')", "'(-1)[a]()'");
+
 shouldBe("compileAndSerialize('new (-1)()')", "'new (-1)()'");
+
+shouldBe("compileAndSerialize('(-1).a')", "'(-1).a'");
+shouldBe("compileAndSerialize('(-1).a = b')", "'(-1).a = b'");
+shouldBe("compileAndSerialize('(-1).a += b')", "'(-1).a += b'");
+shouldBe("compileAndSerialize('(-1).a++')", "'(-1).a++'");
+shouldBe("compileAndSerialize('++(-1).a')", "'++(-1).a'");
+shouldBe("compileAndSerialize('(-1).a()')", "'(-1).a()'");
+
+shouldBe("compileAndSerialize('(- 0)[a]')", "'(- 0)[a]'");
+shouldBe("compileAndSerialize('(- 0)[a] = b')", "'(- 0)[a] = b'");
+shouldBe("compileAndSerialize('(- 0)[a] += b')", "'(- 0)[a] += b'");
+shouldBe("compileAndSerialize('(- 0)[a]++')", "'(- 0)[a]++'");
+shouldBe("compileAndSerialize('++(- 0)[a]')", "'++(- 0)[a]'");
+shouldBe("compileAndSerialize('(- 0)[a]()')", "'(- 0)[a]()'");
+
+shouldBe("compileAndSerialize('new (- 0)()')", "'new (- 0)()'");
+
+shouldBe("compileAndSerialize('(- 0).a')", "'(- 0).a'");
+shouldBe("compileAndSerialize('(- 0).a = b')", "'(- 0).a = b'");
+shouldBe("compileAndSerialize('(- 0).a += b')", "'(- 0).a += b'");
+shouldBe("compileAndSerialize('(- 0).a++')", "'(- 0).a++'");
+shouldBe("compileAndSerialize('++(- 0).a')", "'++(- 0).a'");
+shouldBe("compileAndSerialize('(- 0).a()')", "'(- 0).a()'");
+
+shouldBe("compileAndSerialize('(1)[a]')", "'1[a]'");
+shouldBe("compileAndSerialize('(1)[a] = b')", "'1[a] = b'");
+shouldBe("compileAndSerialize('(1)[a] += b')", "'1[a] += b'");
+shouldBe("compileAndSerialize('(1)[a]++')", "'1[a]++'");
+shouldBe("compileAndSerialize('++(1)[a]')", "'++1[a]'");
+shouldBe("compileAndSerialize('(1)[a]()')", "'1[a]()'");
+
+shouldBe("compileAndSerialize('new (1)()')", "'new 1()'");
+
+shouldBe("compileAndSerialize('(1).a')", "'(1).a'");
+shouldBe("compileAndSerialize('(1).a = b')", "'(1).a = b'");
+shouldBe("compileAndSerialize('(1).a += b')", "'(1).a += b'");
+shouldBe("compileAndSerialize('(1).a++')", "'(1).a++'");
+shouldBe("compileAndSerialize('++(1).a')", "'++(1).a'");
+shouldBe("compileAndSerialize('(1).a()')", "'(1).a()'");
+
+for (i = 0; i < assignmentOperators.length; ++i) {
+    var op = assignmentOperators[i];
+    shouldBe("compileAndSerialize('(-1) " + op + " a')", "'(-1) " + op + " a'");
+    shouldBe("compileAndSerialize('(- 0) " + op + " a')", "'(- 0) " + op + " a'");
+    shouldBe("compileAndSerialize('1 " + op + " a')", "'1 " + op + " a'");
+}
+
 shouldBe("compileAndSerializeLeftmostTest('({ }).x')", "'({ }).x'");
 shouldBe("compileAndSerializeLeftmostTest('x = { }')", "'x = { }'");
 shouldBe("compileAndSerializeLeftmostTest('(function () { })()')", "'(function () { })()'");