Changeset 29815 in webkit for trunk/JavaScriptCore/kjs

Timestamp:

Jan 26, 2008, 8:21:43 PM (17 years ago)

Author:

Darin Adler

Message:

JavaScriptCore:

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.

Location:

trunk/JavaScriptCore/kjs

Files:

• nodes.h (modified) (2 diffs)
• nodes2string.cpp (modified) (7 diffs)

trunk/JavaScriptCore/kjs/nodes.h

@@ -31 +31 @@
 #include "SymbolTable.h"
 #include <wtf/ListRefPtr.h>
+#include <wtf/MathExtras.h>
 #include <wtf/OwnPtr.h>
 #include <wtf/Vector.h>
@@ -253 +254 @@
     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; }

trunk/JavaScriptCore/kjs/nodes2string.cpp

@@ -1 +1 @@
 /*
  *  Copyright (C) 2002 Harri Porten ([email protected])
- *  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 <[email protected]>
  *
@@ -38 +38 @@
 enum IndentType { Indent };
 enum UnindentType { Unindent };
-enum ObjectAccessType { ObjectAccess };
+enum DotExprType { DotExpr };
 
 class SourceStream {
@@ -52 +52 @@
     SourceStream& operator<<(IndentType);
     SourceStream& operator<<(UnindentType);
-    SourceStream& operator<<(ObjectAccessType);
+    SourceStream& operator<<(DotExprType);
     SourceStream& operator<<(Precedence);
     SourceStream& operator<<(const Node*);
@@ -201 +201 @@
     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;
 }
@@ -236 +238 @@
 }
 
-inline SourceStream& SourceStream::operator<<(ObjectAccessType)
+inline SourceStream& SourceStream::operator<<(DotExprType)
 {
     m_numberNeedsParens = true;
@@ -264 +266 @@
 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;
 }
 
@@ -415 +417 @@
 void NewExprNode::streamTo(SourceStream& s) const
 {
-    s << "new " << ObjectAccess << PrecMember << expr << args;
+    s << "new " << PrecMember << expr << args;
 }