Bug 213915 – [JSC] Add exception checks in JSStringBuilder and Array#join

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

[Yusuke Suzuki]

[JSC] Add exception checks in JSStringBuilder and Array#join

[Yusuke Suzuki]

Created attachment 403434 [details]
Patch

[Yusuke Suzuki]

<rdar://problem/64878225>

[Saam Barati]

Comment on attachment 403434 [details]
Patch

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

> Source/JavaScriptCore/runtime/JSStringJoiner.h:169
> +        return;

do we really need this?

[Yusuke Suzuki]

Comment on attachment 403434 [details]
Patch

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

>> Source/JavaScriptCore/runtime/JSStringJoiner.h:169
>> +        return;
> 
> do we really need this?

Since we call `scope.release()`, returning here is better I think. Even if we add a new code after this if branch, then it should work well :)

[Mark Lam]

Comment on attachment 403434 [details]
Patch

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

>>> Source/JavaScriptCore/runtime/JSStringJoiner.h:169
>>> +        return;
>> 
>> do we really need this?
> 
> Since we call `scope.release()`, returning here is better I think. Even if we add a new code after this if branch, then it should work well :)

Could you have done `RELEASE_AND_RETURN(append(jsString->viewWithUnderlyingString(globalObject)));` instead?

[Yusuke Suzuki]

Comment on attachment 403434 [details]
Patch

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

>>>> Source/JavaScriptCore/runtime/JSStringJoiner.h:169
>>>> +        return;
>>> 
>>> do we really need this?
>> 
>> Since we call `scope.release()`, returning here is better I think. Even if we add a new code after this if branch, then it should work well :)
> 
> Could you have done `RELEASE_AND_RETURN(append(jsString->viewWithUnderlyingString(globalObject)));` instead?

Changed.

[Yusuke Suzuki]

Created attachment 403441 [details]
Patch

[EWS]

Committed r263889: <https://trac.webkit.org/changeset/263889>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403441 [details].