Bug 206144

Summary: [JSC] Use internal object field mechanism to implement JSStringIterator
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, hi, joepeck, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch ross.kirsling: review+

Yusuke Suzuki
Reported 2020-01-12 04:19:11 PST
[JSC] Use internal object field mechanism to implement JSStringIterator
Attachments
Patch (27.34 KB, patch)
2020-01-12 04:30 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (27.56 KB, patch)
2020-01-12 04:33 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (28.82 KB, patch)
2020-01-12 14:56 PST, Yusuke Suzuki 		ross.kirsling: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2020-01-12 04:30:53 PST
Created attachment 387466 [details] Patch
Yusuke Suzuki
Comment 2 2020-01-12 04:33:23 PST
Created attachment 387467 [details] Patch
Keith Miller
Comment 3 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?
Yusuke Suzuki
Comment 4 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.
Yusuke Suzuki
Comment 5 2020-01-12 14:56:26 PST
Created attachment 387491 [details] Patch
Ross Kirsling
Comment 6 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?
Yusuke Suzuki
Comment 7 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.
Yusuke Suzuki
Comment 8 2020-01-12 19:12:16 PST
Committed r254420: <https://trac.webkit.org/changeset/254420>
Radar WebKit Bug Importer
Comment 9 2020-01-12 19:13:13 PST
<rdar://problem/58517715>
Note You need to log in before you can comment on or make changes to this bug.