Hi everyone, there is a inconsistency on JSC when we try to get Date.UTC passing only the year param. According to ES6 specs (https://www.ecma-international.org/ecma-262/8.0/index.html#sec-date.utc) the year param is required, the others params are optional. JSC build: 234555 OS: Ubuntu 16.04 x64 Steps to reproduce: d = Date.UTC(2015); print(isNaN(d)); d = Date.UTC(2000 + 15, 0); print(d == 1420070400000); Actual results: true true Expected results: false true Chakra, V8 and SpiderMonkey works as expected. cinfuzz
Created attachment 346904 [details] Patch
Attachment 346904 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] ERROR: Source/JavaScriptCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 346905 [details] Patch
Created attachment 346906 [details] Patch
Comment on attachment 346906 [details] Patch Attachment 346906 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/8819741 New failing tests: ChakraCore.yaml/ChakraCore/test/Date/dateutc.js.default apiTests
Created attachment 346908 [details] Patch
Comment on attachment 346908 [details] Patch r=me. Does this fix any test262 tests?
(In reply to Keith Miller from comment #7) > Comment on attachment 346908 [details] > Patch > > r=me. Does this fix any test262 tests? test/built-ins/Date/UTC/return-value.js has passed! I've added it :D
Committed r234763: <https://trac.webkit.org/changeset/234763>
<rdar://problem/43147473>
Comment on attachment 346908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346908&action=review > Source/JavaScriptCore/runtime/DateConstructor.cpp:94 > + double doubleArguments[7] { > + 0, 0, 1, 0, 0, 0, 0 > + }; Reads nicely on a single line. > Source/JavaScriptCore/runtime/DateConstructor.cpp:95 > + unsigned numberOfUsedArguments = std::max(std::min<unsigned>(7U, args.size()), 1U); I like to write clamping expressions in sequence like this: unsigned numberOfUsedArguments = std::max(1U, std::min<unsigned>(args.size(), 7U)); > Source/JavaScriptCore/runtime/DateConstructor.cpp:101 > + if (!std::isfinite(doubleArguments[i]) || (doubleArguments[i] > INT_MAX) || (doubleArguments[i] < INT_MIN)) This can be done more efficiently without an explicit isfinite check: if (!(doubleArguments[i] >= std::numeric_limits<int>::min() && doubleArguments[i] <= std::numeric_limits<int>::max())) By doing the checks in this fashion, NaN and both infinities are handled naturally without any explicit check. > Source/JavaScriptCore/runtime/DateConstructor.cpp:106 > int year = JSC::toInt32(doubleArguments[0]); Not new to this patch: I don’t think it is correct to call toInt32 after checking if the value is in range. I believe this means the range check doesn’t take the rounding done by toInt32 into account and I suspect we will have incorrect results for values right on the edge of the acceptable range. For example, if the value is the maximum integer + 0.1 or the minimum integer - 0.1 it will be treated as NaN, but should not be. Someone should add test cases to cover these edges and then change this to use the correct idiom. I also suspect that there is a more efficient way to do the JSC::toInt32 operation when we know we don’t want to convert any values outside the 32-bit integer range. For example, maybe std::lround already does the right thing efficiently for that case? Being more efficient here is not critical, but it would be nice to handle the edges 100% correctly.