[webkit-reviews] review granted: [Bug 221380] [JSC] Implement Object.entries in C++ : [Attachment 419230] Patch

[bugzilla-daemon at webkit.org]

Alexey Shvayka <shvaikalesh at gmail.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 221380: [JSC] Implement Object.entries in C++
https://bugs.webkit.org/show_bug.cgi?id=221380

Attachment 419230: Patch

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




--- Comment #3 from Alexey Shvayka <shvaikalesh at gmail.com> ---
Comment on attachment 419230
  --> https://bugs.webkit.org/attachment.cgi?id=419230
Patch

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

Neatly done! r=me with nits.

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:436
> +	   if (propertyName.isSymbol())

Given PropertyNameMode::Strings, I am pretty sure we can assert that
`propertyName` is not a symbol.

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:-410
> -    auto addValue = [&] (PropertyName propertyName) {

Since we are modifying the signature, maybe we should consider more descriptive
name? add{Value,Entry} does not always add a value/entry.