[JSC] Implement Array.prototype.flatMap and Array.prototype.flatten
Created attachment 332978 [details] Patch
Comment on attachment 332978 [details] Patch Attachment 332978 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6333732 Number of test failures exceeded the failure limit.
Created attachment 332980 [details] Archive of layout-test-results from ews113 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 332981 [details] Patch
Comment on attachment 332981 [details] Patch Attachment 332981 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6334626 New failing tests: inspector/model/remote-object-get-properties.html js/Object-getOwnPropertyNames.html
Created attachment 332989 [details] Archive of layout-test-results from ews102 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 332981 [details] Patch Attachment 332981 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6334649 New failing tests: inspector/model/remote-object-get-properties.html js/Object-getOwnPropertyNames.html
Created attachment 332991 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 332981 [details] Patch Attachment 332981 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6334791 New failing tests: js/Object-getOwnPropertyNames.html
Created attachment 332993 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 332981 [details] Patch Attachment 332981 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6335019 New failing tests: js/Object-getOwnPropertyNames.html
Created attachment 332995 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 332981 [details] Patch Attachment 332981 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/6335236 New failing tests: jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout-no-llint jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout-ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout-no-cjit jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout-no-ftl
Comment on attachment 332981 [details] Patch Attachment 332981 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6336081 Number of test failures exceeded the failure limit.
Created attachment 332998 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 333030 [details] Patch
Comment on attachment 333030 [details] Patch Attachment 333030 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6344233 New failing tests: js/Object-getOwnPropertyNames.html
Created attachment 333031 [details] Archive of layout-test-results from ews103 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 333030 [details] Patch Attachment 333030 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6344245 New failing tests: js/Object-getOwnPropertyNames.html
Created attachment 333032 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 333034 [details] Patch
The flatMap tests should include at least one test where the receiver has more than one entry. A very incorrect implementation could still pass these tests.
Created attachment 333036 [details] Patch
(In reply to Michael Ficarra from comment #22) > The flatMap tests should include at least one test where the receiver has > more than one entry. A very incorrect implementation could still pass these > tests. Updated with new tests.
Comment on attachment 333036 [details] Patch Attachment 333036 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6346771 New failing tests: media/modern-media-controls/tracks-support/tracks-support-show-panel-after-dragging-controls.html
Created attachment 333038 [details] Archive of layout-test-results from ews102 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
(In reply to Build Bot from comment #26) > Created attachment 333038 [details] > Archive of layout-test-results from ews102 for mac-sierra > > The attached test failures were seen while running run-webkit-tests on the > mac-ews. > Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6 Itβs unrelated.
Created attachment 333064 [details] Patch
Comment on attachment 333064 [details] Patch Attachment 333064 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/6361756 New failing tests: exceptionFuzz.yaml/exceptionFuzz/3d-cube.js.exception-fuzz
(In reply to Build Bot from comment #29) > Comment on attachment 333064 [details] > Patch > > Attachment 333064 [details] did not pass jsc-ews (mac): > Output: http://webkit-queues.webkit.org/results/6361756 > > New failing tests: > exceptionFuzz.yaml/exceptionFuzz/3d-cube.js.exception-fuzz Is this a real failure?
(In reply to Saam Barati from comment #30) > (In reply to Build Bot from comment #29) > > Comment on attachment 333064 [details] > > Patch > > > > Attachment 333064 [details] did not pass jsc-ews (mac): > > Output: http://webkit-queues.webkit.org/results/6361756 > > > > New failing tests: > > exceptionFuzz.yaml/exceptionFuzz/3d-cube.js.exception-fuzz > > Is this a real failure? This is flakiness. I uploaded the complete same patch for rebasing. And in previous upload, this does not show failures.
Comment on attachment 333064 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333064&action=review > Source/JavaScriptCore/builtins/ArrayPrototype.js:744 > + var result = @arraySpeciesCreate(array, 0); Later, I'll add @arraySpeciesCreate for this patch. > Source/JavaScriptCore/builtins/ArrayPrototype.js:783 > + var result = @arraySpeciesCreate(array, 0); Ditto.
Created attachment 333164 [details] Patch
Comment on attachment 333164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333164&action=review > Source/JavaScriptCore/builtins/ArrayPrototype.js:743 > +function arraySpeciesCreate(array, length) Do we have test coverage for all the special cases in here? > Source/JavaScriptCore/builtins/ArrayPrototype.js:756 > + // We have this check so that if some array from a different global object > + // calls this map they don't get an array with the Array.prototype of the > + // other global object. > + if (arrayConstructorInRealm !== constructor && @isArrayConstructor(constructor)) > + return @newArrayWithSize(length); Test for this case? > Source/JavaScriptCore/builtins/ArrayPrototype.js:781 > + @throwTypeError("flatten array exceeds 2**53 - 1"); This error string looks different in style from some others. Is there some way to test this case, or would the test run too slowly? > Source/JavaScriptCore/builtins/ArrayPrototype.js:800 > + var depthNum = 1; > + var depth = @argument(0); > + if (depth !== @undefined) > + depthNum = @toInteger(depth); I would have expected a ? : for this sort of thing: var depthInteger = depth === @undefined ? 1 : @toInteger(depth); Assuming we generate good code for such expressions. Also makes me wonder if we should make a variety of @toInteger where you can pass a default value in rather than separately doing a check.
Comment on attachment 333164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333164&action=review >> Source/JavaScriptCore/builtins/ArrayPrototype.js:743 >> +function arraySpeciesCreate(array, length) > > Do we have test coverage for all the special cases in here? Added >> Source/JavaScriptCore/builtins/ArrayPrototype.js:756 >> + return @newArrayWithSize(length); > > Test for this case? Added >> Source/JavaScriptCore/builtins/ArrayPrototype.js:781 >> + @throwTypeError("flatten array exceeds 2**53 - 1"); > > This error string looks different in style from some others. > > Is there some way to test this case, or would the test run too slowly? Yeah, currently, we do not test it because it requires too much memory & time (as some of test262 tests are skipped due to this reason). We should explore the effective way to test such things. >> Source/JavaScriptCore/builtins/ArrayPrototype.js:800 >> + depthNum = @toInteger(depth); > > I would have expected a ? : for this sort of thing: > > var depthInteger = depth === @undefined ? 1 : @toInteger(depth); > > Assuming we generate good code for such expressions. Also makes me wonder if we should make a variety of @toInteger where you can pass a default value in rather than separately doing a check. Interestingly, the above code emits more efficient code (of course, it is a bit). @toInteger is a bit costly operation. I'm now planning to introduce op_to_integer, but avoiding this is still a good way I think. (And it is well aligned to the spec description). Before: [ 56] mov loc8, Int32: 1(const0) [ 59] argument loc10, 1 predicting None [ 63] nstricteq loc12, loc10, Undefined(const1) [ 67] jfalse loc12, 30(->97) [ 70] resolve_scope loc16, loc3, PrivateSymbol.toInteger(@id3), <GlobalVar>, 0, 0x7f7071fe0000 [ 77] get_from_scope loc12, loc16, PrivateSymbol.toInteger(@id3), 2049<ThrowIfNotFound|GlobalVar|NotInitialization>, -1298534560 predicting None [ 85] mov loc15, loc10 After: [ 56] argument loc10, 1 predicting None [ 60] stricteq loc12, loc10, Undefined(const0) [ 64] jfalse loc12, 8(->72) [ 67] mov loc8, Int32: 1(const1) [ 70] jmp 29(->99) [ 72] resolve_scope loc16, loc3, PrivateSymbol.toInteger(@id3), <GlobalVar>, 0, 0x7fe402be0000 [ 79] get_from_scope loc12, loc16, PrivateSymbol.toInteger(@id3), 2049<ThrowIfNotFound|GlobalVar|NotInitialization>, 1129965920 predicting None [ 87] mov loc15, loc10 [ 90] call loc8, loc12, 2, 22 (this at loc16) status(Could Take Slow Path) Original; predicting None
Committed r228266: <https://trac.webkit.org/changeset/228266>
<rdar://problem/37347060>