Bug 210023

Summary: [JSC] Implement JSMapIterator/JSSetIterator with JSInternalFieldObjectImpl
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, ews-watchlist, hi, joepeck, jsbell, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 175454    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch keith_miller: review+

Description Yusuke Suzuki 2020-04-05 04:58:16 PDT
[JSC] Implement JSMapIterator with JSInternalFieldObjectImpl
Comment 1 Yusuke Suzuki 2020-04-05 05:00:59 PDT
Created attachment 395500 [details]
Patch
Comment 2 Yusuke Suzuki 2020-04-05 05:13:15 PDT
Created attachment 395501 [details]
Patch
Comment 3 Yusuke Suzuki 2020-04-05 05:27:12 PDT
Created attachment 395503 [details]
Patch
Comment 4 Yusuke Suzuki 2020-04-05 22:41:26 PDT
Created attachment 395546 [details]
Patch
Comment 5 Yusuke Suzuki 2020-04-05 23:06:52 PDT
Created attachment 395548 [details]
Patch
Comment 6 Yusuke Suzuki 2020-04-06 00:48:50 PDT
Created attachment 395552 [details]
Patch
Comment 7 Yusuke Suzuki 2020-04-06 00:57:05 PDT
Created attachment 395553 [details]
Patch
Comment 8 Keith Miller 2020-04-15 14:22:36 PDT
Comment on attachment 395553 [details]
Patch

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

r=me.

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:3225
> +            // Add the constant before exit becomes invalid because we may want to insert (redundant) checks on it in Fixup.
> +            Node* kindNode = jsConstant(jsNumber(static_cast<uint32_t>(kind)));

Why does exit become invalid here? Seems like nothing here has effects should prevent an exit?

> Source/JavaScriptCore/ftl/FTLOperations.cpp:131
> +                target->internalField(static_cast<typename JSCellType::Field>(property.location().info())).set(vm, target, JSValue::decode(values[i]));

Maybe add an ASSERT(property.location().info() < typename JSCellType::numberOfInternalFields);?
Comment 9 Yusuke Suzuki 2020-04-16 06:00:42 PDT
Comment on attachment 395553 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:3225
>> +            Node* kindNode = jsConstant(jsNumber(static_cast<uint32_t>(kind)));
> 
> Why does exit become invalid here? Seems like nothing here has effects should prevent an exit?

Ah, right. Removed the comment.

>> Source/JavaScriptCore/ftl/FTLOperations.cpp:131
>> +                target->internalField(static_cast<typename JSCellType::Field>(property.location().info())).set(vm, target, JSValue::decode(values[i]));
> 
> Maybe add an ASSERT(property.location().info() < typename JSCellType::numberOfInternalFields);?

Sounds nice!
Comment 10 Yusuke Suzuki 2020-04-16 06:07:55 PDT
Committed r260181: <https://trac.webkit.org/changeset/260181>
Comment 11 Radar WebKit Bug Importer 2020-04-16 06:08:25 PDT
<rdar://problem/61883184>
Comment 12 Yusuke Suzuki 2020-04-16 16:30:06 PDT
Committed r260226: <https://trac.webkit.org/changeset/260226>