[webkit-reviews] review granted: [Bug 21203] Optimize appending a number to a string : [Attachment 23900] patch to do it. Some sad code duplication here.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 21203: Optimize appending a number to a string
https://bugs.webkit.org/show_bug.cgi?id=21203

Attachment 23900: patch to do it. Some sad code duplication here.
https://bugs.webkit.org/attachment.cgi?id=23900&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+    if ((leftIsString = v1->isString()) && v2->isString()) {

Why not just put the leftIsString boolean initialization separate in front of
the if statement in a more conventional way?

+	 RefPtr<UString::Rep> value;
+	 if (JSImmediate::isImmediate(v2))
+	     value = concatenate(static_cast<JSString*>(v1)->value().rep(),
JSImmediate::getTruncatedInt32(v2));
+	 else
+	     value = concatenate(static_cast<JSString*>(v1)->value().rep(),
right);

I believe would be more efficient if done with the ternary operator instead of
an if statement. I believe it would save a null check and a branch over deref
code on the assignment to the value local.

+inline size_t expandedSize(size_t size, size_t otherSize)

If this isn't going to be a static member function, then I suggest internal
linkage -- add the static keyword.

+inline bool expandCapacity(UString::Rep* rep, int requiredLength)

This should also be marked static to get internal linkage.

r=me