Normative: explicitly specify order of operations in MakeTime #2120
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While reviewing #2007, @michaelficarra and I noticed that
MakeTime
is underspecified: it has a step which readsbut does not specify in which order to perform the additions. The choice of order is observable when using IEEE double-precision floats: for example,
((80063993375 * 3600000 + 29 * 60000) + 1 * 1000) + -288230376151711744
is29312
, but80063993375 * 3600000 + (29 * 60000 + (1 * 1000 + -288230376151711744))
is29248
.(The actual value is
29256
.) SoDate.UTC
(1970, 0, 1, 80063993375, 29, 1, -288230376151711740)
can have either of those results depending on your choice of order.SpiderMonkey, ChakraCore, V8, and XS all appear to perform the arithmetic in left-to-right order, producing
29312
. Graal produces29248
. (JSC bounds all arguments above by 2^31, so it cannot observe this case, and quickJS produces29256
because it internally uses 64-bit integers for this arithmetic, rather than the IEEE floats required, and so does not lose precision.)In this PR I've chosen to go with the majority and enforce left-to-right evaluation order.