Skip to content

Commit

Permalink
Refactor the Date builtin (#2449)
Browse files Browse the repository at this point in the history
Just a general cleanup of the `Date` builtin to use slightly better patterns and to fix our warnings about deprecated functions.

About the regressed tests. It seems to be a `chrono` bug, so I opened up an issue (chronotope/chrono#884) for it and they've already opened a PR fixing it (chronotope/chrono#885).

However, while checking out the remaining failing tests, I realized there's a more fundamental limitation with the library. Currently, [`chrono`](https://github.com/chronotope/chrono) specifies:

> Date types are limited in about +/- 262,000 years from the common epoch.

While the [ECMAScript spec](https://tc39.es/ecma262/#sec-time-values-and-time-range) says:

> The smaller range supported by a time value as specified in this section is approximately -273,790 to 273,790 years relative to 1970.

The range allowed by the spec is barely outside of the range supported by `chrono`! This is why the remaining `Date` tests fail.
Seeing that, I would like to ping @djc and @esheppa (the maintainers of `chrono`) to ask if it would be feasible to add a feature, akin to the `large-dates` feature from the `time` crate, that expands the supported range of `chrono`.

EDIT: Filed chronotope/chrono#886
  • Loading branch information
jedel1043 committed Nov 22, 2022
1 parent 20db887 commit 1ae4844
Show file tree
Hide file tree
Showing 12 changed files with 1,701 additions and 1,933 deletions.
2,506 changes: 951 additions & 1,555 deletions boa_engine/src/builtins/date/mod.rs

Large diffs are not rendered by default.

711 changes: 424 additions & 287 deletions boa_engine/src/builtins/date/tests.rs

Large diffs are not rendered by default.

200 changes: 200 additions & 0 deletions boa_engine/src/builtins/date/utils.rs
@@ -0,0 +1,200 @@
use chrono::{Datelike, Local, NaiveDateTime, TimeZone, Timelike};

use crate::value::IntegerOrNan;

/// The absolute maximum value of a timestamp
pub(super) const MAX_TIMESTAMP: i64 = 864 * 10i64.pow(13);
/// The number of milliseconds in a second.
pub(super) const MILLIS_PER_SECOND: i64 = 1000;
/// The number of milliseconds in a minute.
pub(super) const MILLIS_PER_MINUTE: i64 = MILLIS_PER_SECOND * 60;
/// The number of milliseconds in an hour.
pub(super) const MILLIS_PER_HOUR: i64 = MILLIS_PER_MINUTE * 60;
/// The number of milliseconds in a day.
pub(super) const MILLIS_PER_DAY: i64 = MILLIS_PER_HOUR * 24;

// https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-time-values-and-time-range
//
// The smaller range supported by a time value as specified in this section is approximately -273,790 to 273,790
// years relative to 1970.
pub(super) const MIN_YEAR: i64 = -300_000;
pub(super) const MAX_YEAR: i64 = -MIN_YEAR;
pub(super) const MIN_MONTH: i64 = MIN_YEAR * 12;
pub(super) const MAX_MONTH: i64 = MAX_YEAR * 12;

/// Calculates the absolute day number from the year number.
pub(super) const fn day_from_year(year: i64) -> i64 {
// Taken from https://chromium.googlesource.com/v8/v8/+/refs/heads/main/src/date/date.cc#496
// Useful to avoid negative divisions and overflows on 32-bit platforms (if we plan to support them).
const YEAR_DELTA: i64 = 399_999;
const fn day(year: i64) -> i64 {
let year = year + YEAR_DELTA;
365 * year + year / 4 - year / 100 + year / 400
}

assert!(MIN_YEAR <= year && year <= MAX_YEAR);
day(year) - day(1970)
}

/// Abstract operation [`MakeTime`][spec].
///
/// [spec]: https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-maketime
pub(super) fn make_time(hour: i64, min: i64, sec: i64, ms: i64) -> Option<i64> {
// 1. If hour is not finite or min is not finite or sec is not finite or ms is not finite, return NaN.
// 2. Let h be 𝔽(! ToIntegerOrInfinity(hour)).
// 3. Let m be 𝔽(! ToIntegerOrInfinity(min)).
// 4. Let s be 𝔽(! ToIntegerOrInfinity(sec)).
// 5. Let milli be 𝔽(! ToIntegerOrInfinity(ms)).

// 6. Let t be ((h * msPerHour + m * msPerMinute) + s * msPerSecond) + milli, performing the arithmetic according to IEEE 754-2019 rules (that is, as if using the ECMAScript operators * and +).
// 7. Return t.

let h_ms = hour.checked_mul(MILLIS_PER_HOUR)?;
let m_ms = min.checked_mul(MILLIS_PER_MINUTE)?;
let s_ms = sec.checked_mul(MILLIS_PER_SECOND)?;

h_ms.checked_add(m_ms)?.checked_add(s_ms)?.checked_add(ms)
}

/// Abstract operation [`MakeDay`][spec].
///
/// [spec]: https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-makeday
pub(super) fn make_day(mut year: i64, mut month: i64, date: i64) -> Option<i64> {
// 1. If year is not finite or month is not finite or date is not finite, return NaN.
// 2. Let y be 𝔽(! ToIntegerOrInfinity(year)).
// 3. Let m be 𝔽(! ToIntegerOrInfinity(month)).
// 4. Let dt be 𝔽(! ToIntegerOrInfinity(date)).
if !(MIN_YEAR..=MAX_YEAR).contains(&year) || !(MIN_MONTH..=MAX_MONTH).contains(&month) {
return None;
}

// At this point, we've already asserted that year and month are much less than its theoretical
// maximum and minimum values (i64::MAX/MIN), so we don't need to do checked operations.

// 5. Let ym be y + 𝔽(floor(ℝ(m) / 12)).
// 6. If ym is not finite, return NaN.
year += month / 12;
// 7. Let mn be 𝔽(ℝ(m) modulo 12).
month %= 12;
if month < 0 {
month += 12;
year -= 1;
}

// 8. Find a finite time value t such that YearFromTime(t) is ym and MonthFromTime(t) is mn and DateFromTime(t) is
// 1𝔽; but if this is not possible (because some argument is out of range), return NaN.
let month = usize::try_from(month).expect("month must be between 0 and 11 at this point");

let mut day = day_from_year(year);

// Consider leap years when calculating the cumulative days added to the year from the input month
if (year % 4 != 0) || (year % 100 == 0 && year % 400 != 0) {
day += [0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334][month];
} else {
day += [0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335][month];
}

// 9. Return Day(t) + dt - 1𝔽.
(day - 1).checked_add(date)
}

/// Abstract operation [`MakeDate`][spec].
///
/// [spec]: https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-makedate
pub(super) fn make_date(day: i64, time: i64) -> Option<i64> {
// 1. If day is not finite or time is not finite, return NaN.
// 2. Let tv be day × msPerDay + time.
// 3. If tv is not finite, return NaN.
// 4. Return tv.
day.checked_mul(MILLIS_PER_DAY)?.checked_add(time)
}

/// Abstract operation [`TimeClip`][spec]
/// Returns the timestamp (number of milliseconds) if it is in the expected range.
/// Otherwise, returns `None`.
///
/// [spec]: https://tc39.es/ecma262/#sec-timeclip
#[inline]
pub(super) fn time_clip(time: i64) -> Option<i64> {
// 1. If time is not finite, return NaN.
// 2. If abs(ℝ(time)) > 8.64 × 10^15, return NaN.
// 3. Return 𝔽(! ToIntegerOrInfinity(time)).
(time.checked_abs()? <= MAX_TIMESTAMP).then_some(time)
}

#[derive(Default, Debug, Clone, Copy)]
pub(super) struct DateParameters {
pub(super) year: Option<IntegerOrNan>,
pub(super) month: Option<IntegerOrNan>,
pub(super) date: Option<IntegerOrNan>,
pub(super) hour: Option<IntegerOrNan>,
pub(super) minute: Option<IntegerOrNan>,
pub(super) second: Option<IntegerOrNan>,
pub(super) millisecond: Option<IntegerOrNan>,
}

/// Replaces some (or all) parameters of `date` with the specified parameters
pub(super) fn replace_params(
datetime: NaiveDateTime,
params: DateParameters,
local: bool,
) -> Option<NaiveDateTime> {
let DateParameters {
year,
month,
date,
hour,
minute,
second,
millisecond,
} = params;

let datetime = if local {
Local.from_utc_datetime(&datetime).naive_local()
} else {
datetime
};

let year = match year {
Some(i) => i.as_integer()?,
None => i64::from(datetime.year()),
};
let month = match month {
Some(i) => i.as_integer()?,
None => i64::from(datetime.month() - 1),
};
let date = match date {
Some(i) => i.as_integer()?,
None => i64::from(datetime.day()),
};
let hour = match hour {
Some(i) => i.as_integer()?,
None => i64::from(datetime.hour()),
};
let minute = match minute {
Some(i) => i.as_integer()?,
None => i64::from(datetime.minute()),
};
let second = match second {
Some(i) => i.as_integer()?,
None => i64::from(datetime.second()),
};
let millisecond = match millisecond {
Some(i) => i.as_integer()?,
None => i64::from(datetime.timestamp_subsec_millis()),
};

let new_day = make_day(year, month, date)?;
let new_time = make_time(hour, minute, second, millisecond)?;
let mut ts = make_date(new_day, new_time)?;

if local {
ts = Local
.from_local_datetime(&NaiveDateTime::from_timestamp_millis(ts)?)
.earliest()?
.naive_utc()
.timestamp_millis();
}

NaiveDateTime::from_timestamp_millis(time_clip(ts)?)
}
26 changes: 12 additions & 14 deletions boa_engine/src/builtins/set/mod.rs
Expand Up @@ -251,20 +251,18 @@ impl Set {
/// [spec]: https://tc39.es/ecma262/#sec-set.prototype.clear
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/clear
pub(crate) fn clear(this: &JsValue, _: &[JsValue], _: &mut Context) -> JsResult<JsValue> {
if let Some(object) = this.as_object() {
if object.borrow().is_set() {
this.set_data(ObjectData::set(OrderedSet::new()));
Ok(JsValue::undefined())
} else {
Err(JsNativeError::typ()
.with_message("'this' is not a Set")
.into())
}
} else {
Err(JsNativeError::typ()
.with_message("'this' is not a Set")
.into())
}
let mut object = this
.as_object()
.map(JsObject::borrow_mut)
.ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Set"))?;

let set = object
.as_set_mut()
.ok_or_else(|| JsNativeError::typ().with_message("'this' is not a Set"))?;

set.clear();

Ok(JsValue::undefined())
}

/// `Set.prototype.delete( value )`
Expand Down
6 changes: 6 additions & 0 deletions boa_engine/src/builtins/set/ordered_set.rs
Expand Up @@ -90,6 +90,12 @@ where
self.inner.shift_remove(value)
}

/// Removes all elements in the set, while preserving its capacity.
#[inline]
pub fn clear(&mut self) {
self.inner.clear();
}

/// Checks if a given value is present in the set
///
/// Return `true` if `value` is present in set, false otherwise.
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/builtins/weak/weak_ref.rs
Expand Up @@ -161,6 +161,6 @@ mod tests {
)
.unwrap(),
JsValue::undefined()
)
);
}
}
1 change: 1 addition & 0 deletions boa_engine/src/context/mod.rs
Expand Up @@ -641,6 +641,7 @@ impl ContextBuilder {
///
/// This function is only available if the `fuzz` feature is enabled.
#[cfg(feature = "fuzz")]
#[must_use]
pub fn instructions_remaining(mut self, instructions_remaining: usize) -> Self {
self.instructions_remaining = instructions_remaining;
self
Expand Down

0 comments on commit 1ae4844

Please sign in to comment.