Skip to content

Fixes for EpochNanoseconds and Offset parsing #223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/builtins/core/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl PlainDateTime {
offset: i64,
calendar: Calendar,
) -> TemporalResult<Self> {
let iso = IsoDateTime::from_epoch_nanos(&instant.as_i128(), offset)?;
let iso = IsoDateTime::from_epoch_nanos(instant.epoch_nanoseconds(), offset)?;
Ok(Self { iso, calendar })
}

Expand Down
10 changes: 5 additions & 5 deletions src/builtins/core/instant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl Instant {
/// Temporal-Proposal equivalent: `AddInstant`.
pub(crate) fn add_to_instant(&self, duration: &TimeDuration) -> TemporalResult<Self> {
let norm = NormalizedTimeDuration::from_time_duration(duration);
let result = self.epoch_nanoseconds() + norm.0;
let result = self.epoch_nanoseconds().0 + norm.0;
Ok(Self::from(EpochNanoseconds::try_from(result)?))
}

Expand Down Expand Up @@ -231,8 +231,8 @@ impl Instant {

/// Returns the `epochNanoseconds` value for this `Instant`.
#[must_use]
pub fn epoch_nanoseconds(&self) -> i128 {
self.as_i128()
pub fn epoch_nanoseconds(&self) -> &EpochNanoseconds {
&self.0
}

// TODO: May end up needing a provider API during impl
Expand Down Expand Up @@ -350,8 +350,8 @@ mod tests {
let max_instant = Instant::try_new(max).unwrap();
let min_instant = Instant::try_new(min).unwrap();

assert_eq!(max_instant.epoch_nanoseconds(), max);
assert_eq!(min_instant.epoch_nanoseconds(), min);
assert_eq!(max_instant.epoch_nanoseconds().0, max);
assert_eq!(min_instant.epoch_nanoseconds().0, min);

let max_plus_one = NS_MAX_INSTANT + 1;
let min_minus_one = NS_MIN_INSTANT - 1;
Expand Down
2 changes: 1 addition & 1 deletion src/builtins/core/timezone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl TimeZone {
provider: &impl TimeZoneProvider,
) -> TemporalResult<IsoDateTime> {
let nanos = self.get_offset_nanos_for(instant.as_i128(), provider)?;
IsoDateTime::from_epoch_nanos(&instant.as_i128(), nanos.to_i64().unwrap_or(0))
IsoDateTime::from_epoch_nanos(instant.epoch_nanoseconds(), nanos.to_i64().unwrap_or(0))
}

/// Get the offset for this current `TimeZoneSlot`.
Expand Down
39 changes: 27 additions & 12 deletions src/builtins/core/zoneddatetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl ZonedDateTime {
.get_iso_datetime_for(&self.instant, provider)?;
// 5. Return ? RoundRelativeDuration(difference, ns2, dateTime, timeZone, calendar, largestUnit, roundingIncrement, smallestUnit, roundingMode).
diff.round_relative_duration(
other.epoch_nanoseconds(),
other.epoch_nanoseconds().as_i128(),
&PlainDateTime::new_unchecked(iso, self.calendar().clone()),
Some((self.timezone(), provider)),
resolved_options,
Expand All @@ -214,7 +214,7 @@ impl ZonedDateTime {
// 3. Let endDateTime be GetISODateTimeFor(timeZone, ns2).
let end = self.tz.get_iso_datetime_for(&other.instant, provider)?;
// 4. If ns2 - ns1 < 0, let sign be -1; else let sign be 1.
let sign = if other.epoch_nanoseconds() - self.epoch_nanoseconds() < 0 {
let sign = if other.epoch_nanoseconds().as_i128() - self.epoch_nanoseconds().as_i128() < 0 {
Sign::Negative
} else {
Sign::Positive
Expand Down Expand Up @@ -254,7 +254,7 @@ impl ZonedDateTime {
)?;
// d. Set timeDuration to TimeDurationFromEpochNanosecondsDifference(ns2, intermediateNs).
time_duration = NormalizedTimeDuration::from_nanosecond_difference(
other.epoch_nanoseconds(),
other.epoch_nanoseconds().as_i128(),
intermediate_ns.0,
)?;
// e. Let timeSign be TimeDurationSign(timeDuration).
Expand Down Expand Up @@ -428,7 +428,7 @@ impl ZonedDateTime {

/// Returns the `epochNanoseconds` value of this `ZonedDateTime`.
#[must_use]
pub fn epoch_nanoseconds(&self) -> i128 {
pub fn epoch_nanoseconds(&self) -> &EpochNanoseconds {
self.instant.epoch_nanoseconds()
}

Expand All @@ -445,13 +445,21 @@ impl ZonedDateTime {
/// Creates a new `ZonedDateTime` from the current `ZonedDateTime`
/// combined with the provided `TimeZone`.
pub fn with_timezone(&self, timezone: TimeZone) -> TemporalResult<Self> {
Self::try_new(self.epoch_nanoseconds(), self.calendar.clone(), timezone)
Self::try_new(
self.epoch_nanoseconds().as_i128(),
self.calendar.clone(),
timezone,
)
}

/// Creates a new `ZonedDateTime` from the current `ZonedDateTime`
/// combined with the provided `Calendar`.
pub fn with_calendar(&self, calendar: Calendar) -> TemporalResult<Self> {
Self::try_new(self.epoch_nanoseconds(), calendar, self.tz.clone())
Self::try_new(
self.epoch_nanoseconds().as_i128(),
calendar,
self.tz.clone(),
)
}

/// Compares one `ZonedDateTime` to another `ZonedDateTime` using their
Expand Down Expand Up @@ -487,8 +495,11 @@ impl ZonedDateTime {
// 10. Else,
// a. Assert: direction is previous.
// b. Let transition be GetNamedTimeZonePreviousTransition(timeZone, zonedDateTime.[[EpochNanoseconds]]).
let transition =
provider.get_named_tz_transition(identifier, self.epoch_nanoseconds(), direction)?;
let transition = provider.get_named_tz_transition(
identifier,
self.epoch_nanoseconds().as_i128(),
direction,
)?;

// 11. If transition is null, return null.
// 12. Return ! CreateTemporalZonedDateTime(transition, timeZone, zonedDateTime.[[Calendar]]).
Expand Down Expand Up @@ -606,7 +617,7 @@ impl ZonedDateTime {
pub fn offset_with_provider(&self, provider: &impl TimeZoneProvider) -> TemporalResult<String> {
let offset = self
.tz
.get_offset_nanos_for(self.epoch_nanoseconds(), provider)?;
.get_offset_nanos_for(self.epoch_nanoseconds().as_i128(), provider)?;
Ok(nanoseconds_to_formattable_offset(offset).to_string())
}

Expand All @@ -616,7 +627,7 @@ impl ZonedDateTime {
) -> TemporalResult<i64> {
let offset = self
.tz
.get_offset_nanos_for(self.epoch_nanoseconds(), provider)?;
.get_offset_nanos_for(self.epoch_nanoseconds().as_i128(), provider)?;
Ok(offset as i64)
}
}
Expand Down Expand Up @@ -1139,6 +1150,7 @@ mod tests {
options::{DifferenceSettings, Disambiguation, OffsetDisambiguation, TemporalUnit},
partial::{PartialDate, PartialTime, PartialZonedDateTime},
primitive::FiniteF64,
time::EpochNanoseconds,
tzdb::FsTzdbProvider,
Calendar, MonthCode, TimeZone,
};
Expand Down Expand Up @@ -1245,10 +1257,13 @@ mod tests {
)
.unwrap();

assert_eq!(start_of_day.epoch_nanoseconds(), -1601753400000000000);
assert_eq!(
start_of_day.epoch_nanoseconds(),
&EpochNanoseconds(-1601753400000000000)
);
assert_eq!(
midnight_disambiguated.epoch_nanoseconds(),
-1601751600000000000
&EpochNanoseconds(-1601751600000000000)
);
let diff = start_of_day
.instant
Expand Down
7 changes: 7 additions & 0 deletions src/epoch_nanoseconds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ impl TryFrom<f64> for EpochNanoseconds {
}
}

// Potential TODO: Build out primitive arthmetic methods if needed.
impl EpochNanoseconds {
pub fn as_i128(&self) -> i128 {
self.0
}
}

/// Utility for determining if the nanos are within a valid range.
#[inline]
#[must_use]
Expand Down
18 changes: 9 additions & 9 deletions src/iso.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use crate::{
utils, TemporalResult, TemporalUnwrap, NS_PER_DAY,
};
use icu_calendar::{Date as IcuDate, Iso};
use num_traits::{cast::FromPrimitive, AsPrimitive, Euclid, ToPrimitive};
use num_traits::{cast::FromPrimitive, AsPrimitive, Euclid};

/// `IsoDateTime` is the record of the `IsoDate` and `IsoTime` internal slots.
#[non_exhaustive]
Expand Down Expand Up @@ -77,19 +77,19 @@ impl IsoDateTime {
// TODO: Move away from offset use of f64
/// Creates an `IsoDateTime` from a `BigInt` of epochNanoseconds.
#[allow(clippy::neg_cmp_op_on_partial_ord)]
pub(crate) fn from_epoch_nanos(nanos: &i128, offset: i64) -> TemporalResult<Self> {
pub(crate) fn from_epoch_nanos(
epoch_nanoseconds: &EpochNanoseconds,
offset: i64,
) -> TemporalResult<Self> {
// Skip the assert as nanos should be validated by Instant.
// TODO: Determine whether value needs to be validated as integral.
// Get the component ISO parts
let mathematical_nanos = nanos.to_i64().ok_or_else(|| {
TemporalError::range().with_message("nanos was not within a valid range.")
})?;

// 2. Let remainderNs be epochNanoseconds modulo 10^6.
let remainder_nanos = mathematical_nanos.rem_euclid(1_000_000);
let remainder_nanos = epoch_nanoseconds.0.rem_euclid(1_000_000);

// 3. Let epochMilliseconds be 𝔽((epochNanoseconds - remainderNs) / 10^6).
let epoch_millis = (mathematical_nanos - remainder_nanos) / 1_000_000;
let epoch_millis = (epoch_nanoseconds.0 - remainder_nanos).div_euclid(1_000_000) as i64;

let (year, month, day) = utils::ymd_from_epoch_milliseconds(epoch_millis);

Expand All @@ -103,11 +103,11 @@ impl IsoDateTime {
let millis = epoch_millis.rem_euclid(1000);

// 11. Let microsecond be floor(remainderNs / 1000).
let micros = remainder_nanos.div_euclid(1000);
let micros = remainder_nanos.div_euclid(1_000) as i64;
// 12. Assert: microsecond < 1000.
temporal_assert!(micros < 1000);
// 13. Let nanosecond be remainderNs modulo 1000.
let nanos = remainder_nanos.rem_euclid(1000);
let nanos = remainder_nanos.rem_euclid(1000) as i64;

Ok(Self::balance(
year,
Expand Down
63 changes: 61 additions & 2 deletions src/parsers/timezone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ pub(crate) fn parse_offset(chars: &mut Peekable<Chars<'_>>) -> TemporalResult<Op
// First offset portion
let hours = parse_digit_pair(chars)?;

if !(0..24).contains(&hours) {
return Err(TemporalError::range().with_message("Invalid offset hour value."));
}

let sep = chars.peek().is_some_and(|ch| *ch == ':');
if sep {
let _ = chars.next();
Expand All @@ -79,7 +83,54 @@ pub(crate) fn parse_offset(chars: &mut Peekable<Chars<'_>>) -> TemporalResult<Op
None => 0,
};

Ok(Some((hours * 60 + minutes) * sign))
if !(0..60).contains(&minutes) {
return Err(TemporalError::range().with_message("Invalid offset hour value."));
}

let result = Some((hours * 60 + minutes) * sign);

// We continue parsing for correctness, but we only care about
// minute precision

let next_peek = chars.peek();
match next_peek {
Some(&':') if sep => _ = chars.next(),
Some(&':') => {
return Err(TemporalError::range().with_message("offset separators do not align."))
}
Some(_) => _ = parse_digit_pair(chars),
None => return Ok(result),
}

let potential_fraction = chars.next();
match potential_fraction {
Some(ch) if ch == '.' || ch == ',' => {
if !chars.peek().is_some_and(|ch| ch.is_ascii_digit()) {
return Err(
TemporalError::range().with_message("fraction separator must have digit after")
);
}
}
Some(_) => return Err(TemporalError::range().with_message("Invalid offset")),
None => return Ok(result),
}

for _ in 0..9 {
let digit_or_end = chars.next().map(|ch| ch.is_ascii_digit());
match digit_or_end {
Some(true) => {}
Some(false) => {
return Err(TemporalError::range().with_message("Not a valid fractional second"))
}
None => break,
}
}

if chars.peek().is_some() {
return Err(TemporalError::range().with_message("Invalid offset"));
}

Ok(result)
}

fn parse_digit_pair(chars: &mut Peekable<Chars<'_>>) -> TemporalResult<i16> {
Expand All @@ -103,7 +154,15 @@ fn parse_digit_pair(chars: &mut Peekable<Chars<'_>>) -> TemporalResult<i16> {
let tens = (first.to_digit(10).expect("validated") * 10) as i16;
let ones = second.to_digit(10).expect("validated") as i16;

Ok(tens + ones)
let result = tens + ones;

if !(0..=59).contains(&result) {
return Err(
TemporalError::range().with_message("digit pair not in a valid range of [0..59]")
);
}

Ok(result)
}

fn parse_iana_component(chars: &mut Peekable<Chars<'_>>) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion temporal_capi/src/instant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub mod ffi {
}

pub fn epoch_nanoseconds(&self) -> I128Nanoseconds {
let ns = self.0.epoch_nanoseconds();
let ns = self.0.epoch_nanoseconds().as_i128();
let is_neg = ns < 0;
let ns = ns.unsigned_abs();

Expand Down