[webkit-reviews] review denied: [Bug 99363] StringImpl::reverseFind() with a single match character isn't optimal for mixed 8/16 bit cases : [Attachment 168778] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 99363: StringImpl::reverseFind() with a single match character isn't
optimal for mixed 8/16 bit cases
https://bugs.webkit.org/show_bug.cgi?id=99363

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

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


> Source/WTF/wtf/text/StringImpl.cpp:1035
> +	   UChar matchCharacter = matchString->is8Bit() ?
matchString->characters8()[0] : matchString->characters16()[0];

StringImpl's operator[] does that for you.

> Source/WTF/wtf/text/StringImpl.cpp:-1135
> -	   if (is8Bit() && matchString->is8Bit())
> -	       return WTF::reverseFind(characters8(), ourLength,
matchString->characters8()[0], index);
> -	   return WTF::reverseFind(characters(), ourLength,
matchString->characters()[0], index);

This was intentional as the reverseFind(LChar*, UChar) was not implemented.
Then I forgot about it.

Does WTF::reverseFind() has this combination now? I don't see it in
StringImpl.h.

> Source/WTF/wtf/text/StringImpl.cpp:1141
> +	   UChar matchCharacter = matchString->is8Bit() ?
matchString->characters8()[0] : matchString->characters16()[0];

see StringImpl's operator[].