Skip to content

Commit f3800d1

Browse files
committed
Address review feedback: Revert day computation in until(), improve min-month calculations for Hebrew and Chinese calendars
1 parent 659278f commit f3800d1

File tree

3 files changed

+28
-52
lines changed

3 files changed

+28
-52
lines changed

components/calendar/src/cal/east_asian_traditional.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,13 @@ impl<R: Rules> DateFieldsResolver for EastAsianTraditional<R> {
587587
12 + year.packed.leap_month().is_some() as u8
588588
}
589589

590+
#[inline]
591+
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> i64 {
592+
// A lunisolar leap month is inserted at least every 3 years. Reingold denies
593+
// that the Chinese calendar determines leap years using the 19-year Metonic cycle.
594+
12 * years + (years / 3)
595+
}
596+
590597
#[inline]
591598
fn extended_year_from_era_year_unchecked(
592599
&self,

components/calendar/src/cal/hebrew.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,9 @@ impl DateFieldsResolver for Hebrew {
135135
}
136136

137137
#[inline]
138-
fn min_months_from_inner(start: HebrewYear, years: i64) -> i64 {
139-
// (7y+1)/19 is the number of leap years before year y. Compute the
140-
// number of leap years in the start year, up to but not including the
141-
// end year.
142-
let count_leaps = |y: i64| (7 * y + 1).div_euclid(19);
143-
let start_year = start.value.into();
144-
let end_year = start_year + years;
145-
12 * years + (count_leaps(end_year) - count_leaps(start_year)).abs()
138+
fn min_months_from_inner(_start: HebrewYear, years: i64) -> i64 {
139+
// There are 7 leap years in every 19-year Metonic cycle.
140+
235 * years / 19
146141
}
147142

148143
#[inline]

components/calendar/src/calendar_arithmetic.rs

Lines changed: 18 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -908,41 +908,6 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
908908
balanced.into_checked().ok_or(DateAddError::Overflow)
909909
}
910910

911-
/// Implements the Temporal operation `NonISODateAdd`, but without any bounds checks.
912-
///
913-
/// For internal use by `until()` where the operation is known to succeed.
914-
/// Follows `Overflow::Constrain` logic.
915-
fn added_without_checks(&self, duration: DateDuration, cal: &C) -> UncheckedArithmeticDate<C> {
916-
let extended_year = duration.add_years_to(self.year().to_extended_year());
917-
let y0 = cal.year_info_from_extended(extended_year);
918-
let base_month = cal.month_from_ordinal(self.year(), self.month());
919-
let m0 = cal
920-
.ordinal_from_month(
921-
y0,
922-
base_month,
923-
DateFromFieldsOptions {
924-
overflow: Some(Overflow::Constrain),
925-
..Default::default()
926-
},
927-
)
928-
.unwrap_or(0);
929-
930-
let end_of_month = Self::new_balanced(y0, duration.add_months_to(m0) + 1, 0, cal);
931-
let base_day = self.day();
932-
let regulated_day = if base_day <= end_of_month.day {
933-
base_day
934-
} else {
935-
end_of_month.day
936-
};
937-
938-
Self::new_balanced(
939-
end_of_month.year,
940-
i64::from(end_of_month.ordinal_month),
941-
duration.add_weeks_and_days_to(regulated_day),
942-
cal,
943-
)
944-
}
945-
946911
/// Implements the Temporal abstract operation `NonISODateUntil`.
947912
///
948913
/// This takes two dates (`self` and `other`), then returns a duration that, when
@@ -1080,16 +1045,25 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
10801045
}
10811046
}
10821047

1083-
// Now that we have `years`, `months`, and `weeks`, we can compute
1084-
// `days` directly by subtracting RD values of the intermediate date
1085-
// (self + YMW) and the target date (`other`).
1086-
let from_date = self.added_without_checks(
1087-
DateDuration::from_signed_ymwd(years, months, weeks, 0),
1048+
// 1. Let _days_ be 0.
1049+
// 1. Let _candidateDays_ be _sign_.
1050+
// 1. Repeat, while NonISODateSurpasses(_calendar_, _sign_, _one_, _two_, _years_, _months_, _weeks_, _candidateDays_) is *false*,
1051+
// 1. Set _days_ to _candidateDays_.
1052+
// 1. Set _candidateDays_ to _candidateDays_ + _sign_.
1053+
let mut days = 0;
1054+
// There is no pressing need to optimize candidate_days here: the early-return RD arithmetic
1055+
// optimization will be hit if the largest_unit is weeks/days, and if it is months or years we will
1056+
// arrive here with a candidate date that is at most 31 days off. We can run this loop 31 times.
1057+
let mut candidate_days = sign;
1058+
while !self.surpasses(
1059+
other,
1060+
DateDuration::from_signed_ymwd(years, months, weeks, candidate_days),
1061+
sign,
10881062
cal,
1089-
);
1090-
let from = C::to_rata_die_inner(from_date.year, from_date.ordinal_month, from_date.day);
1091-
let to = other.to_rata_die();
1092-
let days = to - from;
1063+
) {
1064+
days = candidate_days;
1065+
candidate_days += sign;
1066+
}
10931067

10941068
// 1. Return ! CreateDateDurationRecord(_years_, _months_, _weeks_, _days_).
10951069
DateDuration::from_signed_ymwd(years, months, weeks, days)

0 commit comments

Comments
 (0)