test262 icon indicating copy to clipboard operation
test262 copied to clipboard

Add tests for RoundDuration when unit is "month" and the fractionalDays sign gets flipped

Open anba opened this issue 2 years ago • 2 comments
trafficstars

The sign of fractionalDays can get flipped when adding weeksInDays in step 8.j. Ensure implementations correctly handle this case in the loop condition of step 8.p.

anba avatar Sep 27 '23 08:09 anba

@ptomato The spec polyfill doesn't handle this case correctly.

anba avatar Sep 27 '23 08:09 anba

@anba Thanks for another excellent test case.

In tc39/proposal-temporal#2612 note that we actually are removing this loop: https://github.com/tc39/proposal-temporal/pull/2612/commits/705daa3e4b927c2ef1c60c66f2d7e329e6bd7db3 (commit needs a rebase, but you get the idea.)

The RoundDuration algorithm with unit being "month" will then read:

  1. Let yearsMonths be ! CreateTemporalDuration(years, months, 0, 0, 0, 0, 0, 0, 0, 0).
  2. Let yearsMonthsLater be ? AddDate(calendarRec, plainRelativeTo, yearsMonths).
  3. Let yearsMonthsWeeks be ! CreateTemporalDuration(years, months, weeks, 0, 0, 0, 0, 0, 0, 0).
  4. Let yearsMonthsWeeksLater be ? AddDate(calendarRec, plainRelativeTo, yearsMonthsWeeks).
  5. Let weeksInDays be DaysUntil(yearsMonthsLater, yearsMonthsWeeksLater).
  6. Set plainRelativeTo to yearsMonthsLater.
  7. Set fractionalDays to fractionalDays + weeksInDays.
  8. Let isoResult be ! AddISODate(plainRelativeTo.[[ISOYear]], plainRelativeTo.[[ISOMonth]], plainRelativeTo.[[ISODay]], 0, 0, 0, truncate(fractionalDays), "constrain").
  9. Let wholeDaysLater be ! CreateTemporalDate(isoResult.[[Year]], isoResult.[[Month]], isoResult.[[Day]], calendarRec.[[Receiver]]).
  10. Let untilOptions be OrdinaryObjectCreate(null).
  11. Perform ! CreateDataPropertyOrThrow(untilOptions, "largestUnit", "month").
  12. Let timePassed be ? DifferenceDate(calendarRec, plainRelativeTo, wholeDaysLater, untilOptions).
  13. Let monthsPassed be timePassed.[[Months]].
  14. Set months to months + monthsPassed.
  15. Let monthsPassedDuration be ! CreateTemporalDuration(0, monthsPassed, 0, 0, 0, 0, 0, 0, 0, 0).
  16. Let moveResult be ? MoveRelativeDate(calendarRec, plainRelativeTo, monthsPassedDuration).
  17. Set plainRelativeTo to moveResult.[[RelativeTo]].
  18. Let daysPassed be moveResult.[[Days]].
  19. Set fractionalDays to fractionalDays - daysPassed.
  20. If fractionalDays < 0, let sign be -1; else, let sign be 1.
  21. Let oneMonth be ! CreateTemporalDuration(0, sign, 0, 0, 0, 0, 0, 0, 0, 0).
  22. Let moveResult be ? MoveRelativeDate(calendarRec, plainRelativeTo, oneMonth).
  23. Let oneMonthDays be moveResult.[[Days]].
  24. Let fractionalMonths be months + fractionalDays / abs(oneMonthDays).
  25. Set months to RoundNumberToIncrement(fractionalMonths, increment, roundingMode).
  26. Set total to fractionalMonths.
  27. Set weeks to 0.

At first glance it still seems like it should be possible to cause the condition where fractionalDays is sign-flipped by returning some particular value from dateAdd in the MoveRelativeDate call in what is here numbered step 16.

I can merge this and make sure that the pending PRs accommodate the case correctly, but I'd appreciate your insight on whether my hunch is correct that it'll still be possible to trigger this case after the pending PRs land.

ptomato avatar Sep 27 '23 18:09 ptomato