[webkit-reviews] review granted: [Bug 173414] [JSC] Implement Object.assign in C++ : [Attachment 312979] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 173414: [JSC] Implement Object.assign in C++
https://bugs.webkit.org/show_bug.cgi?id=173414

Attachment 312979: Patch

https://bugs.webkit.org/attachment.cgi?id=312979&action=review




--- Comment #7 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 312979
  --> https://bugs.webkit.org/attachment.cgi?id=312979
Patch

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

r=me

> Source/JavaScriptCore/runtime/JSObjectInlines.h:231
> +ALWAYS_INLINE bool JSObject::putInline(ExecState* exec, PropertyName
propertyName, JSValue value, PutPropertySlot& slot)
> +{
> +    auto putMethod = methodTable(exec->vm())->put;
> +    if (LIKELY(putMethod == JSObject::put))
> +	   return JSObject::putInlineForJSObject(this, exec, propertyName,
value, slot);
> +    return putMethod(this, exec, propertyName, value, slot);
> +}
> +

JSValue::putInline, which calls this, also performs this same branch. Please
remove it from JSValue::putInline since it can just directly call into this
now.

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:326
> +	   RETURN_IF_EXCEPTION(scope, encodedJSValue());

style nit: { } instead of encodedJSValue()
(and below in the function)

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:343
> +

nit: no need to exception check here given how this is used below.