[Webkit-unassigned] [Bug 19408] Constant folding in parser

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #21527|review?                     |review-
               Flag|                            |




------- Comment #3 from darin at apple.com  2008-06-09 06:48 PDT -------
(From update of attachment 21527)
The general approach looks OK.

Here's a quick pass of review.

+    OP_UN_PLUS,         /* '+'   */

For an enum like this, there's no reason to use the ALL_CAPS style. That's
usually reserved for macros, where you're concerned about conflict with other
identifiers, ignoring namespaces. You should also abbreviate less. I think:

    operatorUnaryPlus

is just fine, and OP_UN_PLUS is not really better.

+static ExpressionNode* foldConst(char, ExpressionNode*, ExpressionNode*);

"char" is not the right type to use here. You should give the enum type a name
and use that type. Even if you did want to use a type in the "char" family, it
would probably be unsigned char. Since "char" is signed or unsigned depending
on the compiler.

+                                          if ((en = foldConst(OP_UN_LNOT,
$2.m_node, NULL)))

We use "0" in JavaScriptCore, not NULL.

+        if (typeid(*m1) == typeid(StringNode) && typeid(*m2) ==
typeid(StringNode)) {

There's no RTTI in WebCore so you can't use typeid. If you want to find the
type of a node, you'll need to add an isStringNode() function. But that's a
good thing, because a virtual function call is faster than a typeid check on
most compilers.

+            UString s1 = ((StringNode*)m1)->value();

We use C++ style casts, such as static_cast, not C style casts like this.

+static ExpressionNode* foldConst(char op, ExpressionNode* m1, ExpressionNode*
m2)

I think it would be best to put these functions in a different file rather than
at the bottom of the grammar.

It's critical to use SunSpider and see how this constant folding affects speed
on the benchmark.


-- 
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.