[webkit-reviews] review denied: [Bug 26577] Same warnings in yarr : [Attachment 31603] Kill some warnings.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Laszlo Gombos
<laszlo.1.gombos at nokia.com>'s request for review:
Bug 26577: Same warnings in yarr
https://bugs.webkit.org/show_bug.cgi?id=26577

Attachment 31603: Kill some warnings.
https://bugs.webkit.org/attachment.cgi?id=31603&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Getting rid of these warnings seems good. However, adding C-style typecasts is
not so good.

> -			   while ((++unicodeCurr <= hi) &&
isUnicodeUpper(unicodeCurr) && (Unicode::toLower(unicodeCurr) ==
(lowerCaseRangeEnd + 1)))
> +			   while ((++unicodeCurr <= hi) &&
isUnicodeUpper(unicodeCurr) && (Unicode::toLower(unicodeCurr) ==
(UChar)(lowerCaseRangeEnd + 1)))

Maybe we can avoid the warning here by changing the 1 to a 1U rather than
casting to UChar. Would you try that?

> -	   if (pattern->m_ignoreCase ? ((Unicode::toLower(testChar) == ch) ||
(Unicode::toUpper(testChar) == ch)) : (testChar == ch)) {
> +	   if (pattern->m_ignoreCase ? ((Unicode::toLower(testChar) ==
(UChar32)ch) || (Unicode::toUpper(testChar) == (UChar32)ch)) : (testChar ==
ch)) {

In this case I suggest we change the type of the "ch" local variable to UChar32
from int rather than adding the typecasts. Could you try that?

review- because I'd prefer a version that doesn't add typecasts. If you find my
suggestions don't work feel free to put the original patch up for review again,
but please use static_cast rather than C-style casts.