[webkit-reviews] review granted: [Bug 73995] StringBuilderTest.Append and StringBuilderTest.ToStringPreserveCapacity are failing. : [Attachment 118293] Patch with fix and test re-enabling

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 73995: StringBuilderTest.Append and
StringBuilderTest.ToStringPreserveCapacity are failing.
https://bugs.webkit.org/show_bug.cgi?id=73995

Attachment 118293: Patch with fix and test re-enabling
https://bugs.webkit.org/attachment.cgi?id=118293&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=118293&action=review


> Source/JavaScriptCore/wtf/text/StringBuilder.h:209
> +    mutable unsigned m_valid16BitShadowlen;

I’d spell out length and capitalize it:

    m_valid16BitShadowLength

> Source/JavaScriptCore/wtf/text/StringImpl.cpp:223
>      if (hasTerminatingNullCharacter()) {
> -	   len--;
> -	   m_copyData16[len] = '\0';
> +	   end--;
> +	   m_copyData16[end] = '\0';
>      }

Should this really go inside the helper function? Is this covered by test
cases?

Also, why not just up convert the 8-bit terminating null to the 16-bit
terminating null.