Skip to content

Commit 569f48d

Browse files
committed
review comments
1 parent bd0658b commit 569f48d

File tree

2 files changed

+12
-5
lines changed

2 files changed

+12
-5
lines changed

components/calendar/src/calendar_arithmetic.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,12 @@ where
431431
C: DateFieldsResolver<YearInfo = YearInfo>,
432432
{
433433
// Check that the year is in range to avoid any arithmetic overflow.
434-
range_check(extended_year, "year", -1_000_000..=1_000_000)?;
434+
//
435+
// This range is currently global, but may be replaced with
436+
// a per-calendar check in the future.
437+
//
438+
// <https://github.com/unicode-org/icu4x/issues/7076>
439+
range_check(extended_year, "year", VALID_YEAR_RANGE)?;
435440
Ok(cal.year_info_from_extended(extended_year))
436441
}
437442

@@ -467,7 +472,7 @@ where
467472

468473
if fields.month_code.is_none() && fields.ordinal_month.is_none() {
469474
// We're returning this error early so that we return structural type
470-
// errors before range errors, see comment in Range.
475+
// errors before range errors, see comment in the year code below.
471476
return Err(DateError::NotEnoughFields);
472477
}
473478

components/calendar/src/date.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,11 @@ impl<A: AsCalendar> Date<A> {
149149
/// and fill in missing fields. See [`DateFromFieldsOptions`] for more information.
150150
///
151151
/// This function will not accept year/extended_year values that are outside of the range `[-2²⁷, 2²⁷]`,
152-
/// regardless of the calendar, instead returning a [`DateError::Range`]. This is to protect clients
153-
/// from bugs due to arithmetic overflow. Currently, calendar-specific `Date::try_new_calendarname()` constructors
154-
/// do not do this, though we may change that behavior in the future.
152+
/// regardless of the calendar, instead returning a [`DateError::Range`]. This is to prevent
153+
/// overflowing behaviors near the extreme values of the `i32` range.
154+
/// Currently, calendar-specific `Date::try_new_calendarname()` constructors
155+
/// do not do this, and it is possible to obtain such extreme dates via calendar conversion or arithmetic,
156+
/// though [we may change that behavior in the future](https://github.com/unicode-org/icu4x/issues/7076).
155157
///
156158
/// # Examples
157159
///

0 commit comments

Comments
 (0)