Bug 210023 - [JSC] Implement JSMapIterator/JSSetIterator with JSInternalFieldObjectImpl
Summary: [JSC] Implement JSMapIterator/JSSetIterator with JSInternalFieldObjectImpl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks: 175454
  Show dependency tree / graph
 
Reported: 2020-04-05 04:58 PDT by Yusuke Suzuki
Modified: 2020-04-16 16:30 PDT (History)
12 users (show)

See Also:

Attachments
Patch (79.11 KB, patch)
2020-04-05 05:00 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (77.42 KB, patch)
2020-04-05 05:13 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (83.84 KB, patch)
2020-04-05 05:27 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (117.95 KB, patch)
2020-04-05 22:41 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (118.09 KB, patch)
2020-04-05 23:06 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (123.67 KB, patch)
2020-04-06 00:48 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (123.05 KB, patch)
2020-04-06 00:57 PDT, Yusuke Suzuki 		keith_miller: review+
Details | Formatted Diff | Diff
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>