[webkit-reviews] review granted: [Bug 13388] [js-collector-tweaks] Shrink FunctionImp / DeclaredFunctionImp by 4 bytes : [Attachment 14069] 08-js-gc-function-shrink.patch.txt

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 13388: [js-collector-tweaks] Shrink FunctionImp / DeclaredFunctionImp by 4
bytes
http://bugs.webkit.org/show_bug.cgi?id=13388

Attachment 14069: 08-js-gc-function-shrink.patch.txt
http://bugs.webkit.org/attachment.cgi?id=14069&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+UString FunctionBodyNode::paramString() const
+{
+  UString s;

I think you want to start with an empty string here, rather than a null string.


+  for(ParameterNode *p = param.get(); p != 0L; p = p->nextParam())

Missing a space after for (I know this is copy and paste), and 0L is no better
than 0.

+  for(ParameterNode *p = param.get(); p != 0L; p = p->nextParam())

Again.

+    Parameter() {};

No semicolon here. Do we really need a Parameter type? How about just using
Vector<Identifier>?

+    Identifier paramName(int pos) const { return m_parameters[pos].name; }

Should be size_t rather than int.

r=me