From 1baab80efa3f18a6cc0a7f4a6228214f70a21e08 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:12:43 +0000 Subject: [PATCH] [JSC][Temporal] Count lunisolar months in NonISODateUntil from the day span instead of walking them PlainDate#until and #since with largestUnit "months" in the chinese or dangi calendar cost about 1 ms per month of span: 1.1 s for a century, 20 s for 0001-01-01 to 2034-02-18, all in one native call on the JS thread. NonISODateUntil probed one candidate month at a time, and each probe re-walked the chinese year from M01 to find the ordinal month, about 30 ICU field resolutions with new-moon and solar-term astronomy behind each. hebrew took the same loop with cheap steps, still linear in a span that Temporal allows to reach 6.7 million months. For the lunisolar calendars (chinese, dangi, hebrew), whose months are all 29 or 30 days, lunarMonthsBetween now takes the count of whole months between the two month starts from the epoch-day span over the calendar's mean month, and confirms it by adding that many months in ICU from the start month and landing on the target month start. Month starts never stray from the mean by anything near half a month, so the first probe lands; a probe that misses steps toward the target, and a bracket without a landing is an error. Every candidate short of the target's month stays below it and every one past it surpasses, so this count decides all candidates but the target's own month, which surpasses only on the day, the same rule the fixed-solar closed form in this function already uses. The count no longer needs the ordinal month of either endpoint, which for chinese/dangi is itself a month walk, so the snapshot computes it only for the year loop and the fixed-month paths. Fixed-solar calendars and islamic-* keep their paths through surpassesMonths, which no lunisolar calendar reaches any more. chinese 100 years of months: 1120 ms -> 0.3 ms. 0001-01-01 to 2034-02-18: 20 s -> 0.5 ms. Results are unchanged: a differential corpus of ~90,000 until/since results over eleven calendars matches the old build except for the hebrew case below, and ICU's add(UCAL_MONTH, n) was checked against n single steps for chinese and dangi across the whole +-10000-year range the bridge gives to ICU. The new path adds many months at once for hebrew too, which ran into an ICU bug that calendarDateAdd and the old until tail already hit: since ICU 75, HebrewCalendar::add(UCAL_MONTH, n > 0) fast-forwards whole 19-year cycles by taking multiples of 235 off a month total kept in UCAL_MONTH slots, where the empty Adar I slot of a common year counts too. From Adar..Elul of a common year, a result in Tishri..Adar I lands one month late (5720 Nisan 25 + 463 months gave 5758 Tishri 25 instead of 5757 Elul 25), so add({ months }) was wrong and until() threw or was off by one for hebrew spans over 19 years. addCalendarMonths takes the whole cycles as a year add, where the month slot and its leap status carry over exactly, and leaves ICU a remainder in [-12, 222] that cannot reach the fast path. macOS links the system ICU, so patching the bundled ICU source would not have covered it. Tests: JSTests/stress/temporal-lunar-calendar-until-months-long-span.js JSTests/microbenchmarks/temporal-plain-date-until-lunar-months.js * Source/JavaScriptCore/runtime/temporal/core/CalendarICUBridge.cpp: (JSC::TemporalCore::addCalendarMonths): (JSC::TemporalCore::calendarDateAdd): (JSC::TemporalCore::surpassesMonths): (JSC::TemporalCore::meanLunarMonthDays): (JSC::TemporalCore::lunarMonthsBetween): (JSC::TemporalCore::nonISODateUntil): * JSTests/stress/temporal-lunar-calendar-until-months-long-span.js: Added. * JSTests/microbenchmarks/temporal-plain-date-until-lunar-months.js: Added. --- .../temporal-plain-date-until-lunar-months.js | 31 +++ ...l-lunar-calendar-until-months-long-span.js | 133 ++++++++++++ .../temporal/core/CalendarICUBridge.cpp | 191 +++++++++++++----- 3 files changed, 307 insertions(+), 48 deletions(-) create mode 100644 JSTests/microbenchmarks/temporal-plain-date-until-lunar-months.js create mode 100644 JSTests/stress/temporal-lunar-calendar-until-months-long-span.js diff --git a/JSTests/microbenchmarks/temporal-plain-date-until-lunar-months.js b/JSTests/microbenchmarks/temporal-plain-date-until-lunar-months.js new file mode 100644 index 0000000000000..fc89f94ef1bfa --- /dev/null +++ b/JSTests/microbenchmarks/temporal-plain-date-until-lunar-months.js @@ -0,0 +1,31 @@ +//@ requireOptions("--useTemporal=1") +// Regression benchmark for NonISODateUntil (CalendarICUBridge.cpp) with largestUnit months or +// years in the lunisolar calendars. The month count comes from the epoch-day span, confirmed by +// one ICU month add, so the cost of a difference does not grow with the span; probing one +// candidate month at a time made a century of chinese months cost about a second. + +const cases = []; +for (const calendar of ["chinese", "dangi", "hebrew"]) { + const one = Temporal.PlainDate.from("1924-02-05").withCalendar(calendar); + for (const years of [1, 25, 100]) + cases.push({ one, two: one.add({ years, days: 40 }), largestUnit: "months" }); + cases.push({ one, two: one.add({ years: 100, days: 40 }), largestUnit: "years" }); +} + +for (const { one, two, largestUnit } of cases) { + const forward = one.until(two, { largestUnit }); + if (!one.add(forward).equals(two)) + throw new Error(`Bad difference ${forward} from ${one} to ${two}`); +} + +let months = 0; +for (let i = 0; i < 10; ++i) { + for (const { one, two, largestUnit } of cases) { + months += one.until(two, { largestUnit }).months; + months += two.since(one, { largestUnit }).months; + months += two.until(one, { largestUnit }).months; + } +} + +if (months <= 0) + throw new Error(`Bad total: ${months}`); diff --git a/JSTests/stress/temporal-lunar-calendar-until-months-long-span.js b/JSTests/stress/temporal-lunar-calendar-until-months-long-span.js new file mode 100644 index 0000000000000..91653b35c6758 --- /dev/null +++ b/JSTests/stress/temporal-lunar-calendar-until-months-long-span.js @@ -0,0 +1,133 @@ +//@ requireOptions("--useTemporal=1") + +function shouldBe(actual, expected, message) { + if (actual !== expected) + throw new Error(`${message}: expected ${String(expected)}, got ${String(actual)}`); +} + +// NonISODateUntil (CalendarICUBridge.cpp) counts the months of a lunisolar calendar (chinese, +// dangi, hebrew) from the epoch-day span and confirms the count with one ICU month add, instead of +// probing one candidate month at a time. Everything below compares a result either with the same +// difference computed another way or with a value that does not depend on ICU's data. + +// An independent month count: step one month at a time from the first day of one's month to the +// first day of two's month. Single steps never reach the ICU paths that the span count relies on. +function monthsBySingleSteps(one, two) { + const sign = Temporal.PlainDate.compare(two, one); + let cursor = one.subtract({ days: one.day - 1 }); + const end = two.subtract({ days: two.day - 1 }); + let months = 0; + while (!cursor.equals(end)) { + cursor = sign > 0 ? cursor.add({ months: 1 }) : cursor.subtract({ months: 1 }); + months += sign; + if (sign * Temporal.PlainDate.compare(cursor, end) > 0) + throw new Error(`stepped past ${end} from ${one}`); + } + // The candidate that reaches two's own month surpasses only on the day. + if (months && sign * (one.day - two.day) > 0) + months -= sign; + return months; +} + +function checkDifference(one, two, label, { singleSteps = true } = {}) { + for (const [from, to] of [[one, two], [two, one]]) { + const until = from.until(to, { largestUnit: "months" }); + shouldBe(until.years, 0, `${label} ${from} until ${to} years`); + shouldBe(until.weeks, 0, `${label} ${from} until ${to} weeks`); + // Adding the difference back lands on the other date: the defining property of DateUntil. + shouldBe(from.add(until).equals(to), true, `${label} ${from} + ${until} round trip`); + // Balanced: the day remainder is less than a month. + shouldBe(Math.abs(until.days) < 30, true, `${label} ${from} until ${to} has ${until.days} days`); + // since swaps receiver and argument, so it counts from `to`; the two counts agree whenever + // neither end clamps, which a day-1 endpoint guarantees. + if (from.day === 1 || to.day === 1) + shouldBe(to.since(from, { largestUnit: "months" }).months, until.months, `${label} ${to} since ${from} months`); + if (singleSteps) + shouldBe(until.months, monthsBySingleSteps(from, to), `${label} ${from} until ${to} months by single steps`); + } +} + +for (const calendar of ["chinese", "dangi"]) { + // These took seconds to minutes with one ICU walk per candidate month. A chinese/dangi month + // step is itself slow in ICU4C, so instead of single steps the count is pinned; both ends sit + // mid-month, where no ICU version moves them to another month. + const longSpans = [ + ["0001-01-25", "2034-03-05", 25146], + ["-002000-04-20", "-000001-12-12", 24732], + ["1000-01-01", "1800-01-10", 9894], + ["1899-12-20", "2100-06-20", 2479], + ]; + for (const [a, b, months] of longSpans) { + const one = Temporal.PlainDate.from(a).withCalendar(calendar); + const two = Temporal.PlainDate.from(b).withCalendar(calendar); + shouldBe(one.until(two, { largestUnit: "months" }).months, months, `${calendar} ${a} until ${b} months`); + shouldBe(two.until(one, { largestUnit: "months" }).months, -months, `${calendar} ${b} until ${a} months`); + shouldBe(one.since(two, { largestUnit: "months" }).months, -months, `${calendar} ${a} since ${b} months`); + checkDifference(one, two, calendar, { singleSteps: false }); + } + + // Short spans keep the single-step check; an ICU4C chinese month step is slow, so they stay short. + const shortSpans = [ + ["2023-03-22", "2023-04-20"], // M02 into the leap M02L that follows it + ["2023-02-20", "2024-08-22"], + ["1970-01-31", "1970-03-01"], + ]; + for (const [a, b] of shortSpans) + checkDifference(Temporal.PlainDate.from(a).withCalendar(calendar), Temporal.PlainDate.from(b).withCalendar(calendar), calendar); + + // A day-30 source clamps in the 29-day months along the way, but not in the count. + const day30 = Temporal.PlainDate.from({ calendar, year: 2020, monthCode: "M04", day: 30 }); + shouldBe(day30.day, 30, `${calendar} 2020 M04 has 30 days`); + checkDifference(day30, day30.add({ days: 14 * 29 + 17 }), `${calendar} day 30`); + checkDifference(day30, Temporal.PlainDate.from({ calendar, year: 2021, monthCode: "M09", day: 29 }), `${calendar} day 30 to 29`); +} + +// hebrew: 235 months are exactly 19 years. ICU 75 and later mis-add 229 or more months forward from +// Adar..Elul of a common year when the sum lands in Tishri..Adar I; the bridge routes around that. +{ + const nisan5720 = Temporal.PlainDate.from({ calendar: "hebrew", year: 5720, monthCode: "M07", day: 25 }); + shouldBe(nisan5720.inLeapYear, false, "5720 is a common year"); + const sums = [ + [229, 5739, "M01"], + [235, 5739, "M07"], + [463, 5757, "M12"], + [464, 5758, "M01"], + [470, 5758, "M07"], + [2350, 5910, "M07"], + [2351, 5910, "M08"], + ]; + let stepped = nisan5720; + let steppedMonths = 0; + for (const [months, year, monthCode] of sums) { + const sum = nisan5720.add({ months }); + shouldBe(`${sum.year} ${sum.monthCode} ${sum.day}`, `${year} ${monthCode} 25`, `5720 Nisan 25 + ${months} months`); + shouldBe(sum.subtract({ months }).equals(nisan5720), true, `5720 Nisan 25 + ${months} months and back`); + shouldBe(nisan5720.until(sum, { largestUnit: "months" }).toString(), `P${months}M`, `5720 Nisan 25 until ${sum}`); + for (; steppedMonths < months; ++steppedMonths) + stepped = stepped.add({ months: 1 }); + shouldBe(`${stepped.year} ${stepped.monthCode}`, `${year} ${monthCode}`, `5720 Nisan 25 + ${months} single months`); + } + + const tishri5758 = Temporal.PlainDate.from({ calendar: "hebrew", year: 5758, monthCode: "M01", day: 14 }); + shouldBe(nisan5720.until(tishri5758, { largestUnit: "months" }).toString(), "P463M18D", "5720 Nisan 25 until 5758 Tishri 14"); + shouldBe(tishri5758.since(nisan5720, { largestUnit: "months" }).toString(), "P463M19D", "5758 Tishri 14 since 5720 Nisan 25"); + shouldBe(nisan5720.until(tishri5758, { largestUnit: "years" }).toString(), "P37Y5M18D", "5720 Nisan 25 until 5758 Tishri 14 in years"); + checkDifference(nisan5720, tishri5758, "hebrew"); + + // Two millennia backwards from Adar I 30: the count is pinned instead of single-stepped. + const adarI30 = Temporal.PlainDate.from({ calendar: "hebrew", year: 5784, monthCode: "M05L", day: 30 }); + const heshvan3784 = Temporal.PlainDate.from({ calendar: "hebrew", year: 3784, monthCode: "M02", day: 29 }); + shouldBe(adarI30.until(heshvan3784, { largestUnit: "months" }).toString(), "-P24741M1D", "5784 Adar I 30 until 3784 Heshvan 29"); + shouldBe(heshvan3784.until(adarI30, { largestUnit: "months" }).toString(), "P24741M1D", "3784 Heshvan 29 until 5784 Adar I 30"); + checkDifference(adarI30, heshvan3784, "hebrew Adar I 30", { singleSteps: false }); +} + +// islamic-* months are lunar too but twelve to every year; they share the month-phase tail with the +// lunisolar calendars, so whole years serve as a closed form to check it against. +for (const calendar of ["islamic-civil", "islamic-tbla", "islamic-umalqura"]) { + const one = Temporal.PlainDate.from({ calendar, year: 1380, monthCode: "M01", day: 1 }); + const two = Temporal.PlainDate.from({ calendar, year: 1480, monthCode: "M01", day: 1 }); + shouldBe(one.until(two, { largestUnit: "months" }).toString(), "P1200M", `${calendar} a century in months`); + shouldBe(two.until(one, { largestUnit: "months" }).toString(), "-P1200M", `${calendar} a century in months, backwards`); + checkDifference(Temporal.PlainDate.from("2011-03-30").withCalendar(calendar), Temporal.PlainDate.from("2037-07-07").withCalendar(calendar), calendar); +} diff --git a/Source/JavaScriptCore/runtime/temporal/core/CalendarICUBridge.cpp b/Source/JavaScriptCore/runtime/temporal/core/CalendarICUBridge.cpp index da21b72573379..11e31684b2e84 100644 --- a/Source/JavaScriptCore/runtime/temporal/core/CalendarICUBridge.cpp +++ b/Source/JavaScriptCore/runtime/temporal/core/CalendarICUBridge.cpp @@ -268,6 +268,29 @@ static bool forceICUFieldReresolution(UCalendar* cal) return U_SUCCESS(status); } +// ICU4C-WORKAROUND: ICU 75 and later HebrewCalendar::add(UCAL_MONTH, n > 0) skips whole 19-year +// cycles by taking multiples of 235 off a month total kept in UCAL_MONTH slots, where the empty +// Adar I slot of a common year counts too. From Adar..Elul of a common year, a total that leaves +// fewer than six slots after the skipped cycles lands one month late: 5720 Nisan 25 plus 463 +// months gives 5758 Tishri 25 instead of 5757 Elul 25. Take the whole cycles as years here +// instead (a month slot and its year's leap status both recur every 19 years) and leave ICU a +// remainder in [-12, 222], too small for any slot (at most 12) to reach that path. The year step +// can pin day 30 to 29 in Heshvan or Kislev; every caller regulates the day from the original +// afterwards. Subtraction takes a separate path in ICU that counts correctly. +static bool addCalendarMonths(UCalendar* cal, CalendarID calendarId, int32_t months, UErrorCode& status) +{ + if (calendarId == hebrewCalendarID() && months > 222) { + static constexpr int32_t monthsPerCycle = 235; + static constexpr int32_t yearsPerCycle = 19; + int32_t cycles = static_cast((static_cast(months) + 12) / monthsPerCycle); + ucal_add(cal, UCAL_EXTENDED_YEAR, cycles * yearsPerCycle, &status); + months -= cycles * monthsPerCycle; + } + if (months) + ucal_add(cal, UCAL_MONTH, months, &status); + return U_SUCCESS(status); +} + static TemporalResult validateRejectMode(UCalendar*, CalendarID, std::optional, uint8_t day); // https://tc39.es/proposal-intl-era-monthcode/#sec-temporal-calendarintegerstoiso @@ -1960,8 +1983,7 @@ TemporalResult calendarDateAdd(CalendarID calendarId, const // Add months. ICU balances year rollover implicitly. if (duration.months()) { - ucal_add(cal, UCAL_MONTH, clampTo(duration.months()), &status); - if (U_FAILURE(status)) [[unlikely]] + if (!addCalendarMonths(cal, calendarId, clampTo(duration.months()), status)) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); if (!forceICUFieldReresolution(cal)) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); @@ -2007,7 +2029,8 @@ TemporalResult calendarDateAdd(CalendarID calendarId, const } // surpassesMonths — icu4x: SurpassesChecker::surpasses_months (components/calendar/src/calendar_arithmetic.rs) -// Returns nullopt on ICU failure +// For the calendars with a fixed month count (fixed-solar and islamic-*); the lunisolar ones count +// months with lunarMonthsBetween below. Returns nullopt on ICU failure static std::optional surpassesMonths( UCalendar* trialCal, CalendarID calendarId, @@ -2018,6 +2041,7 @@ static std::optional surpassesMonths( int32_t targetOrdinalMonth, int32_t targetDay) { + ASSERT(!calendarIsLunisolar(calendarId)); UErrorCode status = U_ZERO_ERROR; if (calendarIsNonISOSolar(calendarId)) { auto yearField = calendarArithmeticYearField(calendarId); @@ -2055,21 +2079,66 @@ static std::optional surpassesMonths( if (compareSurpassesLexicographic(sign, trialYear, *trialMonthCode, sourceDay, targetYear, targetMonthCode, targetDay)) return true; - // Phase 2: constrain source monthCode to year y0, compare ordinally. - // trialCal is at the trial position, so its ordinal month equals resolveMonthCodeToOrdinal(calendarId, trialMonthCode, trialYear). - // Read it directly. computeFieldResolutionOrdinalMonth mutates trialCal (walks from year-start - // for lunisolar). save+restore epoch ms preserves the trial position for the next loop iteration. - double savedMs = ucal_getMillis(trialCal, &status); - if (U_FAILURE(status)) [[unlikely]] - return std::nullopt; - auto trialOrdinal = computeFieldResolutionOrdinalMonth(trialCal, calendarId); - UErrorCode restoreStatus = U_ZERO_ERROR; - ucal_setMillis(trialCal, savedMs, &restoreStatus); - if (!trialOrdinal || U_FAILURE(restoreStatus)) [[unlikely]] + // Phase 2: compare ordinally. No leap months here, so the ordinal month is UCAL_MONTH + 1. + auto trialOrdinal = computeOrdinalMonth(trialCal, calendarId); + if (!trialOrdinal) [[unlikely]] return std::nullopt; return compareSurpassesOrdinally(sign, trialYear, *trialOrdinal, sourceDay, targetYear, targetOrdinalMonth, targetDay); } +// Mean days from one month start to the next in the lunisolar calendars, whose months follow the +// moon and are all 29 or 30 days long. Only used to aim lunarMonthsBetween's first probe; ICU +// settles the count. +static double meanLunarMonthDays(CalendarID calendarId) +{ + ASSERT(calendarIsLunisolar(calendarId)); + if (calendarId == hebrewCalendarID()) + return 29 + (12 + 793 / 1080.0) / 24; // The molad interval, 29d 12h 793p (ICU4C HebrewCalendar MONTH_PARTS / DAY_PARTS). + return 29.530588853; // chinese/dangi months run new moon to new moon (ICU4C CalendarAstronomer::SYNODIC_MONTH). +} + +// lunarMonthsBetween — no spec AO; stands in for icu4x's SurpassesChecker::surpasses_months walk +// in the lunisolar calendars (chinese, dangi, hebrew): the signed n for which adding n months +// carries the month starting at monthStartMs to the month starting at targetMonthStartMs, which +// is the number of whole months NonISODateUntil's candidate loop would count one ICU month step at +// a time. Each chinese/dangi step resolves fields through new-moon and solar-term astronomy, so the +// walk cost grew with the span; here the epoch-day span over the mean month gives the count and +// ICU confirms it, normally with the first probe. Leaves cal on the last probed month start. +// Returns nullopt on ICU failure, or if no n lands on targetMonthStartMs (not a month start). +static std::optional lunarMonthsBetween(UCalendar* cal, CalendarID calendarId, double monthStartMs, double targetMonthStartMs) +{ + auto epochDay = [](double epochMs) { + return std::floor(epochMs / millisecondsPerDay); + }; + double targetEpochDay = epochDay(targetMonthStartMs); + double estimate = std::round((targetMonthStartMs - monthStartMs) / (meanLunarMonthDays(calendarId) * millisecondsPerDay)); + // ucal_add takes int32_t; Temporal's date range spans under seven million lunar months. + if (!(std::abs(estimate) < INT32_MAX)) [[unlikely]] + return std::nullopt; + int32_t candidate = static_cast(estimate); + int32_t lastStep = 0; + // Month starts stray from the mean by a few days at most, far less than half a month, so the + // estimate is already the answer; the extra probes only absorb an ICU that disagrees with it. + for (unsigned probe = 0; probe < 8; ++probe) { + UErrorCode status = U_ZERO_ERROR; + ucal_setMillis(cal, monthStartMs, &status); + if (U_FAILURE(status) || !addCalendarMonths(cal, calendarId, candidate, status)) [[unlikely]] + return std::nullopt; + double trialMs = ucal_getMillis(cal, &status); + if (U_FAILURE(status)) [[unlikely]] + return std::nullopt; + double trialEpochDay = epochDay(trialMs); + if (trialEpochDay == targetEpochDay) + return candidate; + int32_t step = trialEpochDay < targetEpochDay ? 1 : -1; + if (lastStep && step != lastStep) [[unlikely]] + return std::nullopt; // Stepped across targetMonthStartMs without landing on it. + lastStep = step; + candidate += step; + } + return std::nullopt; +} + // setMonths — icu4x: SurpassesChecker::set_months (components/calendar/src/calendar_arithmetic.rs) static std::optional setMonths(UCalendar* cal, int32_t sourceDay) { @@ -2091,7 +2160,8 @@ static std::optional setMonths(UCalendar* cal, int32_t sourceDay) // NonISODateUntil — temporal_rs: Calendar::date_until (src/builtins/core/calendar.rs) // temporal_rs delegates to icu4x: AnyCalendar::until -> ArithmeticDate::until + SurpassesChecker (components/calendar/src/calendar_arithmetic.rs) -// ICU4C has no equivalent: fixed-solar calendars balance native fields directly; lunisolar calendars walk months with ucal_add. +// ICU4C has no equivalent: fixed-solar calendars balance native fields directly; lunisolar calendars count months from the +// epoch-day span and confirm the count with ucal_add; islamic-* walks months with ucal_add. // https://tc39.es/proposal-intl-era-monthcode/#sup-temporal-nonisodateuntil static TemporalResult nonISODateUntil(CalendarID calendarId, const ISO8601::PlainDate& one, const ISO8601::PlainDate& two, TemporalUnit largestUnit) { @@ -2104,9 +2174,11 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, std::optional relatedYear; String monthCode; int32_t day { 0 }; - int32_t ordinalMonth { 0 }; + // Engaged only where a caller asked for it: for chinese/dangi it is a month walk from the + // year start, which the lunisolar month count below does without. + std::optional ordinalMonth; }; - auto snapshotFields = [&](const ISO8601::PlainDate& date, bool needsRelatedYear) -> TemporalResult { + auto snapshotFields = [&](const ISO8601::PlainDate& date, bool needsRelatedYear, bool needsOrdinalMonth) -> TemporalResult { return withCalendar(calendarId, [&](UCalendar* cal) -> TemporalResult { if (!cal) [[unlikely]] return makeUnexpected(rangeError(icuOpenCalendarFailed)); @@ -2140,7 +2212,8 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, if (hebrewY0Kislev) { snapshot.monthCode = String("M03"_s); snapshot.day = 30; - snapshot.ordinalMonth = 3; + if (needsOrdinalMonth) + snapshot.ordinalMonth = 3; return snapshot; } auto monthCodeOpt = getMonthCode(cal, calendarId); @@ -2150,20 +2223,24 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, snapshot.day = ucal_get(cal, UCAL_DAY_OF_MONTH, &status); if (U_FAILURE(status)) [[unlikely]] return makeUnexpected(rangeError(icuReadCalendarFailed)); - auto ordinalMonthOpt = computeFieldResolutionOrdinalMonth(cal, calendarId); - if (!ordinalMonthOpt) [[unlikely]] - return makeUnexpected(rangeError(icuReadCalendarFailed)); - snapshot.ordinalMonth = *ordinalMonthOpt; + if (needsOrdinalMonth) { + auto ordinalMonthOpt = computeFieldResolutionOrdinalMonth(cal, calendarId); + if (!ordinalMonthOpt) [[unlikely]] + return makeUnexpected(rangeError(icuReadCalendarFailed)); + snapshot.ordinalMonth = *ordinalMonthOpt; + } return snapshot; }); }; - // Only the source's related year is consumed, and only by the year loop below. - auto targetOrError = snapshotFields(two, false); + // Ordinal months feed the year loop and the fixed-month-count paths; the lunisolar month count + // goes by month starts instead. Only the source's related year is consumed, and only by the year loop. + bool needsOrdinalMonth = largestUnit == TemporalUnit::Year || !calendarIsLunisolar(calendarId); + auto targetOrError = snapshotFields(two, false, needsOrdinalMonth); if (!targetOrError) [[unlikely]] return makeUnexpected(targetOrError.error()); auto& target = *targetOrError; - auto sourceOrError = snapshotFields(one, largestUnit == TemporalUnit::Year); + auto sourceOrError = snapshotFields(one, largestUnit == TemporalUnit::Year, needsOrdinalMonth); if (!sourceOrError) [[unlikely]] return makeUnexpected(sourceOrError.error()); auto& source = *sourceOrError; @@ -2177,11 +2254,11 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, // the target year/month; preserve the existing unregulated source-day comparison. if (calendarIsNonISOSolar(calendarId) && largestUnit == TemporalUnit::Month) { CheckedInt64 checkedMonths = (CheckedInt64(target.year) - source.year) * fixedSolarMonthsInYear(calendarId); - checkedMonths += target.ordinalMonth - source.ordinalMonth; + checkedMonths += *target.ordinalMonth - *source.ordinalMonth; if (checkedMonths.hasOverflowed()) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); int64_t months = checkedMonths; - if (compareSurpassesOrdinally(sign, target.year, target.ordinalMonth, source.day, target.year, target.ordinalMonth, target.day)) + if (compareSurpassesOrdinally(sign, target.year, *target.ordinalMonth, source.day, target.year, *target.ordinalMonth, target.day)) months -= sign; ISO8601::Duration monthDuration; @@ -2207,7 +2284,7 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, if (largestUnit == TemporalUnit::Year) { int64_t candidateYears = minYears ? minYears : sign; while (true) { - auto surpasses = nonISODateSurpasses(calendarId, sign, source.year, *source.relatedYear, source.monthCode, source.day, static_cast(candidateYears), target.year, target.monthCode, target.ordinalMonth, target.day); + auto surpasses = nonISODateSurpasses(calendarId, sign, source.year, *source.relatedYear, source.monthCode, source.day, static_cast(candidateYears), target.year, target.monthCode, *target.ordinalMonth, target.day); if (!surpasses) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); if (*surpasses) @@ -2228,7 +2305,7 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, monthLoopStart = *advanced; } - // Month iteration on the cached calendar. + // Month phase on the cached calendar: count months from (one + years), then days by epoch ms. return withCalendar(calendarId, [&](UCalendar* cal) -> TemporalResult { if (!cal) [[unlikely]] return makeUnexpected(rangeError(icuOpenCalendarFailed)); @@ -2236,28 +2313,47 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, return makeUnexpected(rangeError(icuSetCalendarFailed)); UErrorCode status = U_ZERO_ERROR; - // Lunisolar month counts vary. Fixed-solar calendars reach this loop only for - // largestUnit year, so direct construction is bounded to at most one native year. - int32_t candidateMonths = sign; - // d. Set cal to (one + years) with day=1 for clamping-free month advancement. double startMs = ucal_getMillis(cal, &status); if (U_FAILURE(status)) [[unlikely]] return makeUnexpected(rangeError(icuReadCalendarFailed)); - ucal_set(cal, UCAL_DAY_OF_MONTH, 1); - ucal_getMillis(cal, &status); // force ICU state resolution - if (U_FAILURE(status)) [[unlikely]] - return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); - for (;;) { - auto surpasses = surpassesMonths(cal, calendarId, sign, source.day, target.year, target.monthCode, target.ordinalMonth, target.day); - if (!surpasses) [[unlikely]] + if (calendarIsLunisolar(calendarId)) { + // Every candidate short of the target's own month stays below it and every one past it + // surpasses, whatever the days, so the count of whole months from (one + years)'s month + // to the target's month decides every candidate but one: the target's month itself + // surpasses only on CompareSurpasses' day key. + int32_t startDay = ucal_get(cal, UCAL_DAY_OF_MONTH, &status); + if (U_FAILURE(status)) [[unlikely]] + return makeUnexpected(rangeError(icuReadCalendarFailed)); + double monthStartMs = startMs - (startDay - 1) * millisecondsPerDay; + double targetMonthStartMs = target.epochMs - (target.day - 1) * millisecondsPerDay; + auto wholeMonths = lunarMonthsBetween(cal, calendarId, monthStartMs, targetMonthStartMs); + if (!wholeMonths) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); - if (*surpasses) - break; - months = candidateMonths; - candidateMonths += sign; - // cal already advanced by surpassesMonths — no reset needed here. - // ucal_setMillis(cal, startMs) below resets it before applying the final months count. + months = *wholeMonths; + if (months && sign * (source.day - target.day) > 0) + months -= sign; + } else { + // Fixed-solar calendars reach this loop only for largestUnit year, so direct + // construction is bounded to at most one native year; islamic-* walks with ucal_add. + int32_t candidateMonths = sign; + // d. Set cal to (one + years) with day=1 for clamping-free month advancement. + ucal_set(cal, UCAL_DAY_OF_MONTH, 1); + ucal_getMillis(cal, &status); // force ICU state resolution + if (U_FAILURE(status)) [[unlikely]] + return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); + + for (;;) { + auto surpasses = surpassesMonths(cal, calendarId, sign, source.day, target.year, target.monthCode, *target.ordinalMonth, target.day); + if (!surpasses) [[unlikely]] + return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); + if (*surpasses) + break; + months = candidateMonths; + candidateMonths += sign; + // cal already advanced by surpassesMonths — no reset needed here. + // ucal_setMillis(cal, startMs) below resets it before applying the final months count. + } } // Restore cal to (one + years), then apply total months without undoing trial steps. @@ -2279,8 +2375,7 @@ static TemporalResult nonISODateUntil(CalendarID calendarId, if (!advancedExactly || !*advancedExactly) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); } else { - ucal_add(cal, UCAL_MONTH, months, &status); - if (U_FAILURE(status)) [[unlikely]] + if (!addCalendarMonths(cal, calendarId, months, status)) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed)); if (!forceICUFieldReresolution(cal)) [[unlikely]] return makeUnexpected(rangeError(icuCalendarArithmeticFailed));