[Webkit-unassigned] [Bug 17017] Remove KJS::UChar

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=17017


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18754|review?                     |review+
               Flag|                            |




------- Comment #16 from darin at apple.com  2008-01-29 12:56 PDT -------
(From update of attachment 18754)
 552       if (charLen && (u == 0 || u >= 128 || !strchr(do_not_unescape,
static_cast<unsigned char>(u)))) {

There's no need for a cast here. strchr takes an int and the earlier code on
the line already checks that it's a value in the range 1-127, so a normal
conversion to int will do.

 837             if (Lexer::isHexDigit((c + 2)[0]) && Lexer::isHexDigit((c +
3)[0]) && Lexer::isHexDigit((c + 4)[0]) && Lexer::isHexDigit((c + 5)[0])) {
 838                 u = Lexer::convertUnicode((c + 2)[0], (c + 3)[0], (c +
4)[0], (c + 5)[0]);

Really should be c[2] instead of (c + 2)[0]. The [0] is a good trick to do this
with a global replace, but it's a little ugly.

 116             d[i] = static_cast<unsigned char>(c[i]); // static_cast to
unsigned char to avoid erroneous sign-extend

The comment says the sign extend here would be "erroneous", but really I think
that this function is only called on pure ASCII strings. While changing 128-255
into U+0080 to U+00FF is arguably better than changing them to U+FF80 to
U+FFFF, I think what's really going on here is that it's never going to matter.
I think a better comment would simply state that we use unsigned char because
we want to zero-extend rather than sign extending. Note that cross-platform,
plain old "char" can be *either* unsigned or signed.

 97 void Lexer::setCode(int startingLineNumber, const UChar *c, unsigned int
len)

Surprised you didn't move the star.

266212     UString &append(const char *);
267213     UString &append(unsigned short);
268214     UString &append(char c) { return append(static_cast<unsigned
short>(static_cast<unsigned char>(c))); }
269      UString &append(UChar c) { return append(c.uc); }

I think we need the append(unsigned short) to be changed to be an
append(UChar), because on Windows those two types aren't the same. As-is I
think this might break the Windows build.

r=me on this as-is, but please make sure you don't break the Windows build


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.