Skip to content

Commit 7689416

Browse files
Manishearthrobertbastian
authored andcommitted
More documentation for min_months_from_inner (unicode-org#7746)
Improving docs from unicode-org#7739 --------- Co-authored-by: Robert Bastian <4706271+robertbastian@users.noreply.github.com>
1 parent 6ad1c09 commit 7689416

File tree

3 files changed

+30
-4
lines changed

3 files changed

+30
-4
lines changed

components/calendar/src/cal/east_asian_traditional.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,7 @@ impl<R: Rules> DateFieldsResolver for EastAsianTraditional<R> {
589589

590590
#[inline]
591591
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.
592+
// TODO(#7077) This is not verified, and the error is not constant bounded
594593
12 * years + (years / 3)
595594
}
596595

components/calendar/src/cal/hebrew.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,30 @@ impl DateFieldsResolver for Hebrew {
136136

137137
#[inline]
138138
fn min_months_from_inner(_start: HebrewYear, years: i64) -> i64 {
139-
// There are 7 leap years in every 19-year Metonic cycle.
139+
// The Hebrew Metonic cycle is 7 leap years every 19 years,
140+
// which comes out to 235 months per 19 years.
141+
//
142+
// We need to ensure that this is always *lower or equal to* the number of
143+
// months in a given year span.
144+
//
145+
// Firstly, note that this math will produce exactly the number of months in any given period
146+
// that spans a whole number of cycles. Note that we are only performing integer
147+
// ops here, and our SAFE_YEAR_RANGE is well within the range of allowed values
148+
// for multiplying by 235.
149+
//
150+
// So we only need to verify that this math produces the right results within a single cycle.
151+
//
152+
// The Hebrew Metonic cycle has leap years in year 3, 6, 8, 11, 14, 17, and 19 (starting counting at year 1),
153+
// i.e., leap year gaps of +3, +3, +2, +3, +3, +3, +2.
154+
//
155+
// 235 / 19 is ≈12 7/19 months per year, which leads to one leap month every three years plus 2/19
156+
// months left over. So this is correct as long as it does not predict a leap month in 2 years
157+
// where the Hebrew calendar expects one in 3.
158+
//
159+
// The longest sequence of "three year leap months" in the Hebrew calendar
160+
// is 4: year 8->11->14->17. In that time the error will accumulate to 6/19, which is not
161+
// enough to create a "two year leap month" in our calculation. So this calculation cannot go past
162+
// the actual cycle of the Hebrew calendar.
140163
235 * years / 19
141164
}
142165

components/calendar/src/calendar_arithmetic.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,11 @@ pub(crate) trait DateFieldsResolver: Calendar {
235235
12
236236
}
237237

238-
/// The minimum number of months over `years` years, starting from the given year.
238+
/// A lower bound for the number of months in `years` years, starting from the beginning of this year.
239+
///
240+
/// This may be equal to the number of months, but should never be larger. Ideally,
241+
/// implementations should be close enough to the actual answer such that the error
242+
/// is bounded by a constant with respect to `years`.
239243
///
240244
/// The default impl is for non-lunisolar calendars with 12 months!
241245
#[inline]

0 commit comments

Comments
 (0)