Bug 206144 - [JSC] Use internal object field mechanism to implement JSStringIterator
Summary: [JSC] Use internal object field mechanism to implement JSStringIterator
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:
 
Reported: 2020-01-12 04:19 PST by Yusuke Suzuki
Modified: 2020-01-12 19:13 PST (History)
10 users (show)

See Also:

Attachments
Patch (27.34 KB, patch)
2020-01-12 04:30 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (27.56 KB, patch)
2020-01-12 04:33 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (28.82 KB, patch)
2020-01-12 14:56 PST, Yusuke Suzuki 		ross.kirsling: review+
Details | Formatted Diff | Diff
Show Obsolete (2) 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-01-12 04:19:11 PST 
[JSC] Use internal object field mechanism to implement JSStringIterator
Comment 1 Yusuke Suzuki 2020-01-12 04:30:53 PST 
Created attachment 387466 [details]
Patch
Comment 2 Yusuke Suzuki 2020-01-12 04:33:23 PST 
Created attachment 387467 [details]
Patch
Comment 3 Keith Miller 2020-01-12 11:53:50 PST 
Seems like there's some weird test failures. It's not obvious to me why your patch would break those tests. Maybe, you just incidentally surfaced the issue?
Comment 4 Yusuke Suzuki 2020-01-12 14:48:56 PST 
(In reply to Keith Miller from comment #3)
> Seems like there's some weird test failures. It's not obvious to me why your
> patch would break those tests. Maybe, you just incidentally surfaced the
> issue?

Found that some array-iterator related symbols are used for a private variable name. Fixed the test.
Comment 5 Yusuke Suzuki 2020-01-12 14:56:26 PST 
Created attachment 387491 [details]
Patch
Comment 6 Ross Kirsling 2020-01-12 18:01:35 PST 
Comment on attachment 387491 [details]
Patch

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

r=me, looks straightforward.

> Source/JavaScriptCore/builtins/BuiltinNames.h:-53
> -    macro(arrayIteratorNextIndex) \
> -    macro(arrayIterationKind) \
> -    macro(arrayIteratorNext) \
> -    macro(arrayIteratorIsDone) \
> -    macro(arrayIteratorKind) \

These are leftover from r254252, I take it?

> JSTests/stress/tailCallForwardArguments.js:6
> -var putFuncToPrivateName = createBuiltin(`(function (func) { @arrayIteratorIsDone = func })`)
> +var putFuncToPrivateName = createBuiltin(`(function (func) { @generatorThis = func })`)

Ahh, so this test had just arbitrarily chosen to write to a private name you're removing?
Comment 7 Yusuke Suzuki 2020-01-12 19:11:47 PST 
Comment on attachment 387491 [details]
Patch

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

>> Source/JavaScriptCore/builtins/BuiltinNames.h:-53
>> -    macro(arrayIteratorKind) \
> 
> These are leftover from r254252, I take it?

Right.

>> JSTests/stress/tailCallForwardArguments.js:6
>> +var putFuncToPrivateName = createBuiltin(`(function (func) { @generatorThis = func })`)
> 
> Ahh, so this test had just arbitrarily chosen to write to a private name you're removing?

Right.
Comment 8 Yusuke Suzuki 2020-01-12 19:12:16 PST 
Committed r254420: <https://trac.webkit.org/changeset/254420>
Comment 9 Radar WebKit Bug Importer 2020-01-12 19:13:13 PST 
<rdar://problem/58517715>