[webkit-reviews] review granted: [Bug 23175] String and UString should be able to share a UChar* buffer. : [Attachment 26618] Addressed comments -- Part 2: Refactor UString::Rep into two classes which reduces memory usage for the base Rep class.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted David Levin <levin at chromium.org>'s
request for review:
Bug 23175: String and UString should be able to share a UChar* buffer.
https://bugs.webkit.org/show_bug.cgi?id=23175

Attachment 26618: Addressed comments -- Part 2: Refactor UString::Rep into two
classes which reduces memory usage for the base Rep class.
https://bugs.webkit.org/attachment.cgi?id=26618&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    COMPILE_ASSERT(numCharactersToStore ==
sizeof(SmallStrings::m_singleCharacterStrings) /
sizeof(SmallStrings::m_singleCharacterStrings[0]), 
> +		      WTF_IsNumCharactersConstInSyncWithClassUsage);

No need to put that WTF_ prefix on the COMPILE_ASSERT argument. That name is
only used for a local symbol, which can't conflict with anything outside this
function. The only place you *might* want that prefix would be if you used
COMPILE_ASSERT in a header, and even there I don't think it’s needed.

Do you need those SmallStrings:: qualifiers? I'd think this would compile
without them since we’re inside a SmallStrings constructor definition.

> +	       void setBaseString(PassRefPtr<BaseString> base);

This is a classic case where we would not name the argument in the function
declaration, since the argument's type and the function name make it perfectly
clear without a name.

r=me