Bug 140139 – Investigate the character type of repeated string instead of checking is8Bit flag

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Investigate the character type of repeated string instead of checking is8Bit flag

[Yusuke Suzuki]

Created attachment 244082 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 244082 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244082&action=review

In this patch, I've fixed the issue pointed at [1].

1: https://bugs.webkit.org/show_bug.cgi?id=140047#c22

> Source/JavaScriptCore/runtime/StringPrototype.cpp:679
> +static inline JSValue repeatCharacter(ExecState* exec, CharacterType character, unsigned repeatCount)

Change name to `repeatCharacter`.

> Source/JavaScriptCore/runtime/StringPrototype.cpp:725
> +        if (!(character & ~0xff))

Checking character is 8bit or not. This check is the same to the guart which is used in the StringImpl / StringBuiler[1].

1: http://trac.webkit.org/browser/trunk/Source/WTF/wtf/text/StringImpl.h#L1135

[Yusuke Suzuki]

Created attachment 244087 [details]
Patch

[Darin Adler]

Comment on attachment 244087 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244087&action=review

> Source/JavaScriptCore/runtime/StringPrototype.cpp:727
> +            return JSValue::encode(repeatCharacter<LChar>(exec, static_cast<LChar>(character), repeatCount));
> +        return JSValue::encode(repeatCharacter<UChar>(exec, character, repeatCount));

I don’t think the <LChar> and <UChar> are needed after repeatCharacter; I think template type deduction will take care of this.

[Yusuke Suzuki]

Comment on attachment 244087 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244087&action=review

Thanks for your review :)

>> Source/JavaScriptCore/runtime/StringPrototype.cpp:727
>> +        return JSValue::encode(repeatCharacter<UChar>(exec, character, repeatCount));
> 
> I don’t think the <LChar> and <UChar> are needed after repeatCharacter; I think template type deduction will take care of this.

Agreed. `static_cast` ensures the instantiated result of the template function.
I'll fix it and update the patch.

[Yusuke Suzuki]

Created attachment 244154 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 244154 [details]
Patch

Clearing flags on attachment: 244154

Committed r178098: <http://trac.webkit.org/changeset/178098>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yusuke Suzuki]

Thank you for your r+ and cq+ :D