Bug 221380 – [JSC] Implement Object.entries in C++

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

[Yusuke Suzuki]

[JSC] Implement Object.entries in C++

[Yusuke Suzuki]

*** Bug 221233 has been marked as a duplicate of this bug. ***

[Yusuke Suzuki]

Created attachment 419230 [details]
Patch

[Alexey Shvayka]

Comment on attachment 419230 [details]
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.

[Alexey Shvayka]

Comment on attachment 419230 [details]
Patch

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

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:434
> +    for (unsigned i = 0, numProperties = properties.size(); i < numProperties; i++) {

I wonder if it's measurably faster than using `for (const auto& propertyName : properties)`?

[Yusuke Suzuki]

Comment on attachment 419230 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:434
>> +    for (unsigned i = 0, numProperties = properties.size(); i < numProperties; i++) {
> 
> I wonder if it's measurably faster than using `for (const auto& propertyName : properties)`?

I don't think there is any perf difference. Changed to for (const auto& propertyName : properties).

>> 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.

Right! Fixed.

>> 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.

OK, changed.

[Yusuke Suzuki]

Created attachment 419234 [details]
Patch

[Yusuke Suzuki]

Created attachment 419235 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 419235 [details]
Patch

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

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:429
> +        MarkedArgumentBuffer args;
> +        args.append(jsOwnedString(vm, propertyName.uid()));
> +        args.append(value);
> +        JSArray* entry = constructArray(globalObject, static_cast<ArrayAllocationProfile*>(nullptr), args);

Slightly optimized it with

        {
            ObjectInitializationScope initializationScope(vm);
            if ((entry = JSArray::tryCreateUninitializedRestricted(initializationScope, nullptr, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous), 2))) {
                entry->initializeIndex(initializationScope, 0, key);
                entry->initializeIndex(initializationScope, 1, value);
            }
        }
        if (!entry) {
            throwOutOfMemoryError(globalObject, scope);
            return;
        }

[Yusuke Suzuki]

Created attachment 419238 [details]
Patch

[EWS]

Committed r272364: <https://trac.webkit.org/changeset/272364>

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

[Radar WebKit Bug Importer]

<rdar://problem/73973248>