[webkit-reviews] review denied: [Bug 108338] Add a StringTypeAdapter for ASCIILiteral : [Attachment 185520] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Christophe Dumez
<dchris at gmail.com>'s request for review:
Bug 108338: Add a StringTypeAdapter for ASCIILiteral
https://bugs.webkit.org/show_bug.cgi?id=108338

Attachment 185520: Patch
https://bugs.webkit.org/attachment.cgi?id=185520&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=185520&action=review


Thank you for fixing this.

Can you please also extend the tests:
Tools/TestWebKitAPI/Tests/WTF/StringOperators.cpp?

> Source/WTF/wtf/text/StringConcatenate.h:274
> +    StringTypeAdapter<ASCIILiteral>(const char* buffer)

I would not rely on the implicit conversion for the constructor. I think an
explicit conversion would read better.

> Source/WTF/wtf/text/StringConcatenate.h:292
> +	   for (unsigned i = 0; i < m_length; ++i)
> +	       destination[i] = m_buffer[i];

We have copyChars() in StringImpl to do exactly this. :)