Skip to content

Commit 8ebf5ed

Browse files
committed
Merge remote-tracking branch 'upstream/main' into stateful-surpasses
# Conflicts: # components/calendar/src/calendar_arithmetic.rs
2 parents 08fc7bd + e337ba5 commit 8ebf5ed

File tree

5 files changed

+65
-25
lines changed

5 files changed

+65
-25
lines changed

components/calendar/src/cal/coptic.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// called LICENSE at the top level of the ICU4X source tree
33
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
44

5-
use crate::calendar_arithmetic::ArithmeticDate;
6-
use crate::calendar_arithmetic::DateFieldsResolver;
5+
use crate::calendar_arithmetic::{ArithmeticDate, DateFieldsResolver, MinMonths};
76
use crate::error::{
87
DateAddError, DateError, DateFromFieldsError, EcmaReferenceYearError, UnknownEraError,
98
};
@@ -68,8 +67,9 @@ impl DateFieldsResolver for Coptic {
6867
}
6968

7069
#[inline]
71-
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> i64 {
72-
13 * years
70+
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> MinMonths {
71+
// This calendar is defined as having 13 months per year, this never changes.
72+
MinMonths::Guaranteed(13 * years)
7373
}
7474

7575
#[inline]

components/calendar/src/cal/east_asian_traditional.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
// called LICENSE at the top level of the ICU4X source tree
33
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
44

5-
use crate::calendar_arithmetic::{ArithmeticDate, ToExtendedYear};
6-
use crate::calendar_arithmetic::{DateFieldsResolver, PackWithMD};
5+
use crate::calendar_arithmetic::{
6+
ArithmeticDate, DateFieldsResolver, MinMonths, PackWithMD, ToExtendedYear,
7+
};
78
use crate::error::{
89
DateAddError, DateError, DateFromFieldsError, EcmaReferenceYearError, LunisolarDateError,
910
MonthError, UnknownEraError,
@@ -609,9 +610,17 @@ impl<R: Rules> DateFieldsResolver for EastAsianTraditional<R> {
609610
}
610611

611612
#[inline]
612-
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> i64 {
613-
// TODO(#7077) This is not verified, and the error is not constant bounded
614-
12 * years + (years / 3)
613+
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> MinMonths {
614+
// TODO(#7077) Try to turn this into a guarantee
615+
MinMonths::Guessed {
616+
// This is a somewhat conservative guess that leads to an unbounded
617+
// error for larger and larger year spans.
618+
guess: 12 * years + (years / 3),
619+
// We can guarantee 12 months per year, this is a fundamental property
620+
// of East Asian Traditional calendars and assumed in the behavior of
621+
// EastAsianTraditionalYear
622+
guarantee: 12 * years,
623+
}
615624
}
616625

617626
#[inline]

components/calendar/src/cal/ethiopian.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
use crate::cal::coptic::CopticDateInner;
66
use crate::cal::Coptic;
7-
use crate::calendar_arithmetic::{ArithmeticDate, DateFieldsResolver};
7+
use crate::calendar_arithmetic::{ArithmeticDate, DateFieldsResolver, MinMonths};
88
use crate::error::{
99
DateAddError, DateError, DateFromFieldsError, EcmaReferenceYearError, UnknownEraError,
1010
};
@@ -95,7 +95,7 @@ impl DateFieldsResolver for Ethiopian {
9595
}
9696

9797
#[inline]
98-
fn min_months_from_inner(start: Self::YearInfo, years: i64) -> i64 {
98+
fn min_months_from_inner(start: Self::YearInfo, years: i64) -> MinMonths {
9999
Coptic::min_months_from_inner(start, years)
100100
}
101101

components/calendar/src/cal/hebrew.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// called LICENSE at the top level of the ICU4X source tree
33
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
44

5-
use crate::calendar_arithmetic::{ArithmeticDate, DateFieldsResolver, PackWithMD, ToExtendedYear};
5+
use crate::calendar_arithmetic::{
6+
ArithmeticDate, DateFieldsResolver, MinMonths, PackWithMD, ToExtendedYear,
7+
};
68
use crate::error::{
79
DateAddError, DateError, DateFromFieldsError, EcmaReferenceYearError, LunisolarDateError,
810
MonthError, UnknownEraError,
@@ -135,7 +137,7 @@ impl DateFieldsResolver for Hebrew {
135137
}
136138

137139
#[inline]
138-
fn min_months_from_inner(_start: HebrewYear, years: i64) -> i64 {
140+
fn min_months_from_inner(_start: HebrewYear, years: i64) -> MinMonths {
139141
// The Hebrew Metonic cycle is 7 leap years every 19 years,
140142
// which comes out to 235 months per 19 years.
141143
//
@@ -160,7 +162,7 @@ impl DateFieldsResolver for Hebrew {
160162
// is 4: year 8->11->14->17. In that time the error will accumulate to 6/19, which is not
161163
// enough to create a "two year leap month" in our calculation. So this calculation cannot go past
162164
// the actual cycle of the Hebrew calendar.
163-
235 * years / 19
165+
MinMonths::Guaranteed(235 * years / 19)
164166
}
165167

166168
#[inline]

components/calendar/src/calendar_arithmetic.rs

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,17 @@ impl PackWithMD for i32 {
183183
}
184184
}
185185

186+
/// A lower bound for the number of months in `years` years, for use in `until`.
187+
pub(crate) enum MinMonths {
188+
/// This number is guaranteed to be correct based on documented checking of invariants
189+
///
190+
/// It is ok for ICU4X to `debug_assert` on things deriving from the correctness of this calculation
191+
Guaranteed(i64),
192+
/// This number is not guaranteed. Code should check that it is not too large, and if not,
193+
/// fall back to the `guarantee` value (which *is* guaranteed).
194+
Guessed { guess: i64, guarantee: i64 },
195+
}
196+
186197
/// Trait for converting from era codes, month codes, and other fields to year/month/day ordinals.
187198
pub(crate) trait DateFieldsResolver: Calendar {
188199
/// This stores the year as either an i32, or a type containing more
@@ -242,9 +253,13 @@ pub(crate) trait DateFieldsResolver: Calendar {
242253
/// is bounded by a constant with respect to `years`.
243254
///
244255
/// The default impl is for non-lunisolar calendars with 12 months!
256+
///
257+
/// `until()` will debug assert if this ever returns a value greater than the
258+
/// month diff betweeen two dates as a Guarantee. If such a value is returned as a Guess,
259+
/// it will simply be slow
245260
#[inline]
246-
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> i64 {
247-
12 * years
261+
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> MinMonths {
262+
MinMonths::Guaranteed(12 * years)
248263
}
249264

250265
/// Calculates the ordinal month for the given year and month code.
@@ -923,7 +938,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
923938
cal: &C,
924939
options: DateDifferenceOptions,
925940
) -> DateDuration {
926-
// Fast path for day/week diffs
941+
// Non-spec optimization: fast path for day/week diffs
927942
// Avoids quadratic behavior in surpasses() for days/weeks
928943
if matches!(
929944
options.largest_unit,
@@ -949,6 +964,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
949964

950965
let mut surpasses_checker = SurpassesChecker::new(self, other, sign, cal);
951966

967+
// Preparation for non-specced optimization:
952968
// We don't want to spend time incrementally bumping it up one year
953969
// at a time, so let's pre-guess a year delta that is guaranteed to not
954970
// surpass.
@@ -976,9 +992,10 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
976992
let mut years = 0;
977993
if matches!(options.largest_unit, Some(DateDurationUnit::Years)) {
978994
let mut candidate_years = sign;
995+
996+
// Non-spec optimization: try to fast-forward candidate_years to
997+
// a year value that does not surpass
979998
if min_years != 0 {
980-
// Optimization: we start with min_years since it is guaranteed to not
981-
// surpass.
982999
candidate_years = min_years
9831000
};
9841001

@@ -1002,13 +1019,25 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
10021019
Some(DateDurationUnit::Years) | Some(DateDurationUnit::Months)
10031020
) {
10041021
let mut candidate_months = sign;
1022+
1023+
// Non-spec optimization: try to fast-forward candidate_months to
1024+
// a month value that does not surpass
10051025
if options.largest_unit == Some(DateDurationUnit::Months) && min_years != 0 {
10061026
// If largest_unit = Months, then compute the calendar-specific minimum number of
1007-
// months corresponding to min_years. For solar calendars, this is 12 * min_years.
1008-
// For the Hebrew calendar, a leap month is added for 7 out of 19 years. East Asian
1009-
// Calendars do not provide a specialized implementation of `min_months_from()`
1010-
// because it would be too expensive to calculate; they default to 12 * min_years.
1011-
let min_months = self.min_months_from(min_years);
1027+
// months corresponding to min_years.
1028+
let min_months = match self.min_months_from(min_years) {
1029+
MinMonths::Guaranteed(m) => m,
1030+
// In case it's a guess, check that the guess is in range,
1031+
// and if it's not, return the guarantee
1032+
MinMonths::Guessed { guess, guarantee } => {
1033+
if surpasses_checker.surpasses_months(guess) {
1034+
// surpasses, so it's not in range
1035+
guarantee
1036+
} else {
1037+
guess
1038+
}
1039+
}
1040+
};
10121041

10131042
// clippy rejected: debug_assert!(!surpasses_checker.surpasses_months(min_months));
10141043
#[cfg(debug_assertions)]
@@ -1066,7 +1095,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
10661095
}
10671096

10681097
/// The minimum number of months over `years` years, starting from `self.year()`.
1069-
pub(crate) fn min_months_from(self, years: i64) -> i64 {
1098+
pub(crate) fn min_months_from(self, years: i64) -> MinMonths {
10701099
C::min_months_from_inner(self.year(), years)
10711100
}
10721101
}

0 commit comments

Comments
 (0)