Bug 141054

<rdar://problem/19654521>

Created attachment 245658 [details] Patch

Comment on attachment 245658 [details] Patch Attachment 245658 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6287579481636864 New failing tests: js/Object-getOwnPropertyNames.html

Created attachment 245673 [details] Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5

Comment on attachment 245658 [details] Patch Attachment 245658 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5152604308897792 New failing tests: js/Object-getOwnPropertyNames.html

Created attachment 245683 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5

Comment on attachment 245658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245658&action=review > Source/JavaScriptCore/builtins/ArrayConstructor.js:29 > + return typeof fn === 'function' || @Object.prototype.toString.@call(fn) === '[object Function]'; For isCallable, I would just use the typeof check. @Object.prototype is problematic -- but luckily, we can just delete it. > Source/JavaScriptCore/builtins/ArrayConstructor.js:32 > + var toLength = function(value) { Are we allowed to define our helper functions outside of from()? That would be more efficient. Otherwise, you'll allocate two function objects per call to from(). > Source/JavaScriptCore/builtins/ArrayConstructor.js:36 > + if (@isNaN(numberValue)) { You can just use "numberValue != numberValue" instead of @isNaN if you want.

Comment on attachment 245658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245658&action=review >> Source/JavaScriptCore/builtins/ArrayConstructor.js:29 >> + return typeof fn === 'function' || @Object.prototype.toString.@call(fn) === '[object Function]'; > > For isCallable, I would just use the typeof check. > > @Object.prototype is problematic -- but luckily, we can just delete it. Great. >> Source/JavaScriptCore/builtins/ArrayConstructor.js:32 >> + var toLength = function(value) { > > Are we allowed to define our helper functions outside of from()? That would be more efficient. Otherwise, you'll allocate two function objects per call to from(). I don't think we can call helpers outside the built-ins, because these functions are not really defined in a global scope. The script parses the file looking for top-level functions and squirts the JS code directly into the C++, then intercepts the call. So I think I'll just inline the functions. There really isn't a need in both cases here. >> Source/JavaScriptCore/builtins/ArrayConstructor.js:36 >> + if (@isNaN(numberValue)) { > > You can just use "numberValue != numberValue" instead of @isNaN if you want. Nice! That saves another global.

Created attachment 246924 [details] Patch

Comment on attachment 246924 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246924&action=review > Source/JavaScriptCore/builtins/ArrayConstructor.js:59 > + var itemsLength = lengthValue > 0 ? (lengthValue < maxSafeInteger ? lengthValue : maxSafeInteger) : 0; Is this really the rule? In all other respects, the max value of an array's length is (1 << 31) - 2.

Comment on attachment 246924 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246924&action=review >> Source/JavaScriptCore/builtins/ArrayConstructor.js:59 >> + var itemsLength = lengthValue > 0 ? (lengthValue < maxSafeInteger ? lengthValue : maxSafeInteger) : 0; > > Is this really the rule? In all other respects, the max value of an array's length is (1 << 31) - 2. This is what the spec wants!

Committed r180370: <http://trac.webkit.org/changeset/180370>

Comment on attachment 246924 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246924&action=review > LayoutTests/js/script-tests/array-from.js:69 > +var crazyPants = { Best var evar.

Comment on attachment 246924 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246924&action=review > Source/JavaScriptCore/builtins/ArrayConstructor.js:49 > + var numberValue = @Number(items.length); If "arrayLike" was a Map/Set then we would want .size instead of .length. Perhaps that is why this doesn't work for Set. I think there is a generic way to check if something is iterable and get an iterator via Symbol.iterator: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols So this would work with any custom iterable object. That said, I don't know if we implement those portions.

The new test fails on all bots: ** The following JSC stress test failures have been introduced: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl-no-cjit jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-cjit jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint https://build.webkit.org/builders/Apple%20Mavericks%20Release%20WK2%20(Tests)/builds/11730/steps/jscore-test/logs/stdio Dean, are you available to take a look soon?

Re-opening to attach a prospective test fix.

Created attachment 246939 [details] test fix Removing document.querySelectorAll use. It is not appropriate in LayoutTests/js, these cannot rely on DOM. This probably need to be re-added in a separate test.

Comment on attachment 246939 [details] test fix Clearing flags on attachment: 246939 Committed r180394: <http://trac.webkit.org/changeset/180394>

(In reply to comment #18) > Comment on attachment 246939 [details] > test fix > > Clearing flags on attachment: 246939 > > Committed r180394: <http://trac.webkit.org/changeset/180394> There are still some failing tests on iOS 64 bit after 180394: Test r180388 r180394 jsc-layout-tests.yaml/js/script-tests/array-from.js.layout Failed Passed jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-dfg-eager-no-cjit Failed Passed jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl Failed Passed jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl-eager-no-cjit Failed Passed jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl-no-cjit Failed Failed jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-cjit Failed Failed jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint Failed Failed All of the failures look like this: [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: DIFF FAILURE! [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: --- ../.tests/jsc-layout-tests.yaml/js/array-from-expected.txt 2015-02-22 14:50:14.000000000 -0800 [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: +++ ../jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint.out 2015-02-22 15:16:04.000000000 -0800 [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: @@ -58,11 +58,11 @@ [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: Modify length during construction [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: ------- [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: -PASS Array.from(crazyPants) is ['one', 'two', 'three', 'four'] [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: +FAIL Array.from(crazyPants) should be one,two,three,four. Was one,two,three,four,ERROR: this should never be called. [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: Modify length during mapping [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: ------- [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: -PASS Array.from(crazyPants, function (x) { crazyPants.length = x; return x; }) is ['one', 'two', 'three', 'four'] [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: +FAIL Array.from(crazyPants, function (x) { crazyPants.length = x; return x; }) should be one,two,three,four. Was one,two,three,four,ERROR: this should never be called. [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: Construction using Set object [2015-02-22 15:16:05] INFO: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint: ------- [2015-02-22 15:16:05] ERROR: FAIL: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint

note: It is skipped by http://trac.webkit.org/changeset/180573. Additionally I get exactly the same failure on Linux Aarch64, but it didn't fail in all cases: FAIL: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-llint FAIL: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-no-cjit FAIL: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl-no-cjit PASS: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout PASS: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-dfg-eager-no-cjit PASS: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl PASS: jsc-layout-tests.yaml/js/script-tests/array-from.js.layout-ftl-eager-no-cjit

Array.from has been working for a while. There is a separate issue about arm64 which has its own bug filed. Closing this.