[Webkit-unassigned] [Bug 17214] Regression: Extraneous parentheses in function.toString()

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=17214


rgovostes at gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rgovostes at gmail.com
                URL|http://rgov.org/            |http://rgov.org/safaritest.h
                   |                            |tml




------- Comment #1 from rgovostes at gmail.com  2008-02-07 22:57 PDT -------
I added some debug info to my nodes2string.h to help trace this:

function foo()
[ScopeNode]
        {
        [VarStatementNode]
                var
                [CommaNode]
                        (
                        [CommaNode]
                                [AssignResolveNode]
                                x = 
                                        [StringNode]
                                                "a"
                                        [/StringNode]
                                [/AssignResolveNode]
                                , 
                                [AssignResolveNode]
                                        y = 
                                        [StringNode]
                                                "b"
                                        [/StringNode]
                                [/AssignResolveNode]
                        [/CommaNode]
                        ), 
                        [AssignResolveNode]
                                z = 
                                [StringNode]
                                        "c"
                                [/StringNode]
                        [/AssignResolveNode]
                [/CommaNode]
                ;
        [/VarStatementNode]
}[/ScopeNode]

It looks like where the first faulty parenthesis is going to be added, the
needsParens check in SourceStream::operator<<(const Node* n) is true because

1. m_precedence = 17
2. n->precedence() = 18

In other words, it's because the CommaNode's precedence is greater than the
AssignResolveNode.

Someone please double check this for me but I think the function should be
rewritten as:

-----
 SourceStream& SourceStream::operator<<(const Node* n)
    {
        if (!n) {
            m_precedence = PrecExpression;
            return *this;
        }

        bool needParens = (m_precedence < PrecAssignment && n->precedence() >
m_precedence) || (m_atStartOfStatement && n->needsParensIfLeftmost());
        // and so on
-----

(I moved the n == NULL check to the top because I think it could cause a NULL
pointer dereference as it stands in the current build.)

Furthermore, a test case should be added in the parenthesization tests to
ensure this problem does not creep up again.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.