Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
e88e66e
Use bnum to define an InternalDuration struct
robot-head Jun 22, 2025
93f5315
Finish addressing all implications
robot-head Jun 22, 2025
ece5d8f
Merge branch 'main' into ms/optimize_duration
robot-head Jun 22, 2025
c546ae3
Remove TimeDuration completely
robot-head Jun 22, 2025
9c4b638
Merge branch 'ms/optimize_duration' of https://github.com/robot-head/…
robot-head Jun 22, 2025
22a2d41
Fix compile error
robot-head Jun 22, 2025
5cfb206
Fix lint
robot-head Jun 22, 2025
87c8513
Fix tests
robot-head Jun 22, 2025
21db4f5
Fix tests and lint errors
robot-head Jun 22, 2025
45d3262
Fix depcheck
robot-head Jun 22, 2025
a2e677a
cargo run -p diplomat-gen
robot-head Jun 22, 2025
9d46a14
Fix sign calculations
robot-head Jun 22, 2025
cbd388e
Fix compile error
robot-head Jun 22, 2025
0f67ff8
Fix duration sign handling and time diff
robot-head Jun 22, 2025
1a78f43
Merge pull request #6 from robot-head/codex/fix-failing-tests
robot-head Jun 22, 2025
f22a1d1
Fix duration sign handling
robot-head Jun 22, 2025
25ed765
Fix lint
robot-head Jun 22, 2025
6b9b646
Merge pull request #7 from robot-head/6o0p22-codex/fix-failing-tests
robot-head Jun 22, 2025
7123b03
Fix lint
robot-head Jun 22, 2025
79cdb11
Fix duration calc
robot-head Jun 22, 2025
9fd53fe
Respond to comments
robot-head Jun 23, 2025
7aa9d0f
Merge remote-tracking branch 'origin/main' into ms/optimize_duration
robot-head Jul 4, 2025
c0eecb0
Post-merge fixes
robot-head Jul 4, 2025
74442e4
Fix sign on NormalizedDurationRecord
robot-head Jul 5, 2025
12abb62
Fix nudge_to_day_or_time
robot-head Jul 5, 2025
06e9f9d
Add from_components method to NormalizedTimeDuration for creating ins…
Jul 31, 2025
ee85fa0
Refactor U48 type definition to use BUintD16
Jul 31, 2025
ef1f029
Refactor Duration constructor to use absolute values for time components
Jul 31, 2025
4ee26b4
Merge branch 'main' into ms/optimize_duration
nekevss Aug 3, 2025
0e72c84
Fix broken tests + update various abstract operations
nekevss Aug 3, 2025
3d14622
Update the FFI
nekevss Aug 3, 2025
7895f65
Update tzif-inspect + add test case to from_partial_duration
nekevss Aug 3, 2025
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
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ timezone_provider = { workspace = true, optional = true }
# System time feature
web-time = { workspace = true, optional = true }
iana-time-zone = { workspace = true, optional = true }
bnum = "0.13.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: hmm, I know this was @nekevss's suggestion, but I'm not really excited about adding a new dependency just for an optimization, especially one we haven't measured the benefit of)

suggestion: Can we implement one of the earlier suggestions to use narrow stdlib integer types and flatten Duration, and then have a separate discussion for adding the bnum dependency? I imagine that switchover will be cleaner.

@nekevss opinions?

(Given that this is reviewed already, I wouldn't recommend changing the PR until we decide what we want here, it's possible temporal_rs decides to take on the bnum dep)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, the one thing I've been most iffy about is adding the extra dependency (to the point of debating looking up how to manually implement arbitrarily sized integers 😅)

Hmmmmmm, I can flatten the struct and do some of the specification updates in a separate PR. If I realized that was going to be such a big portion of this PR, then I would've wanted that split off in a different PR anyways. My general concern is that the resulting Duration type is going to be huge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a stack type, huge isn't really a big deal most of the time.

For JS implementors it will likely be heap allocated. But it's not like people are making thousands of these, I think.


[features]
default = ["sys"]
Expand Down
39 changes: 32 additions & 7 deletions src/builtins/compiled/duration/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
OffsetDisambiguation, RelativeTo, RoundingIncrement, RoundingMode, RoundingOptions, Unit,
},
partial::PartialDuration,
Calendar, DateDuration, PlainDate, TimeDuration, TimeZone, ZonedDateTime,
Calendar, DateDuration, PlainDate, TimeZone, ZonedDateTime,
};

use core::{num::NonZeroU32, str::FromStr};
Expand Down Expand Up @@ -373,7 +373,20 @@ fn basic_negative_expand_rounding() {

#[test]
fn rounding_to_fractional_day_tests() {
let twenty_five_hours = Duration::from(TimeDuration::new(25, 0, 0, 0, 0, 0).unwrap());
let twenty_five_hours = Duration::new_unchecked(
Default::default(),
0,
0,
0,
0u8.into(),
25u8.into(),
0u8.into(),
0u8.into(),
0u8.into(),
0u8.into(),
0u8.into(),
);

let options = RoundingOptions {
largest_unit: Some(Unit::Day),
smallest_unit: None,
Expand All @@ -383,7 +396,7 @@ fn rounding_to_fractional_day_tests() {
let result = twenty_five_hours.round(options, None).unwrap();
assert_duration(result, (0, 0, 0, 1, 1, 0, 0, 0, 0, 0));

let sixty_days = Duration::from(DateDuration::new_unchecked(0, 0, 0, 64));
let sixty_days = Duration::from(DateDuration::new(0, 0, 0, 64).unwrap());
let options = RoundingOptions {
largest_unit: None,
smallest_unit: Some(Unit::Day),
Expand All @@ -393,7 +406,7 @@ fn rounding_to_fractional_day_tests() {
let result = sixty_days.round(options, None).unwrap();
assert_duration(result, (0, 0, 0, 60, 0, 0, 0, 0, 0, 0));

let sixty_days = Duration::from(DateDuration::new_unchecked(0, 0, 0, 64));
let sixty_days = Duration::from(DateDuration::new(0, 0, 0, 64).unwrap());
let options = RoundingOptions {
largest_unit: None,
smallest_unit: Some(Unit::Day),
Expand All @@ -403,7 +416,7 @@ fn rounding_to_fractional_day_tests() {
let result = sixty_days.round(options, None).unwrap();
assert_duration(result, (0, 0, 0, 60, 0, 0, 0, 0, 0, 0));

let sixty_days = Duration::from(DateDuration::new_unchecked(0, 0, 0, 64));
let sixty_days = Duration::from(DateDuration::new(0, 0, 0, 64).unwrap());
let options = RoundingOptions {
largest_unit: None,
smallest_unit: Some(Unit::Day),
Expand All @@ -413,7 +426,7 @@ fn rounding_to_fractional_day_tests() {
let result = sixty_days.round(options, None).unwrap();
assert_duration(result, (0, 0, 0, 70, 0, 0, 0, 0, 0, 0));

let sixty_days = Duration::from(DateDuration::new_unchecked(0, 0, 0, 1000));
let sixty_days = Duration::from(DateDuration::new(0, 0, 0, 1000).unwrap());
let options = RoundingOptions {
largest_unit: None,
smallest_unit: Some(Unit::Day),
Expand Down Expand Up @@ -485,7 +498,19 @@ fn basic_subtract_duration() {
// days-24-hours-relative-to-zoned-date-time.js
#[test]
fn round_relative_to_zoned_datetime() {
let duration = Duration::from(TimeDuration::new(25, 0, 0, 0, 0, 0).unwrap());
let duration = Duration::new_unchecked(
Default::default(),
0,
0,
0,
0u8.into(),
25u8.into(),
0u8.into(),
0u8.into(),
0u8.into(),
0u8.into(),
0u8.into(),
);
let zdt = ZonedDateTime::try_new(
1_000_000_000_000_000_000,
Calendar::default(),
Expand Down
6 changes: 2 additions & 4 deletions src/builtins/core/calendar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
//! Temporal compatible calendar implementations.

use crate::{
builtins::core::{
duration::DateDuration, Duration, PlainDate, PlainDateTime, PlainMonthDay, PlainYearMonth,
},
builtins::core::{Duration, PlainDate, PlainDateTime, PlainMonthDay, PlainYearMonth},
iso::IsoDate,
options::{ArithmeticOverflow, Unit},
parsers::parse_allowed_calendar_formats,
TemporalError, TemporalResult,
DateDuration, TemporalError, TemporalResult,
};
use alloc::string::ToString;
use core::str::FromStr;
Expand Down
74 changes: 24 additions & 50 deletions src/builtins/core/date.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
//! This module implements `Date` and any directly related algorithms.

use crate::{
builtins::core::{
calendar::Calendar, duration::DateDuration, Duration, PlainDateTime, PlainTime,
ZonedDateTime,
},
builtins::core::{calendar::Calendar, Duration, PlainDateTime, PlainTime, ZonedDateTime},
iso::{IsoDate, IsoDateTime, IsoTime},
options::{
ArithmeticOverflow, DifferenceOperation, DifferenceSettings, Disambiguation,
DisplayCalendar, ResolvedRoundingOptions, Unit, UnitGroup,
},
parsers::{parse_date_time, IxdtfStringBuilder},
provider::{NeverProvider, TimeZoneProvider},
MonthCode, TemporalError, TemporalResult, TemporalUnwrap, TimeZone,
DateDuration, MonthCode, TemporalError, TemporalResult, TemporalUnwrap, TimeZone,
};
use alloc::{format, string::String};
use core::{cmp::Ordering, str::FromStr};
Expand All @@ -21,8 +18,7 @@ use ixdtf::records::DateRecord;
use writeable::Writeable;

use super::{
calendar::month_to_month_code,
duration::{normalized::NormalizedDurationRecord, TimeDuration},
calendar::month_to_month_code, duration::normalized::NormalizedDurationRecord,
PartialYearMonth, PlainMonthDay, PlainYearMonth,
};
use tinystr::TinyAsciiStr;
Expand Down Expand Up @@ -265,53 +261,31 @@ impl PlainDate {
Self { iso, calendar }
}

// Updated: 2025-08-03
/// Returns the date after adding the given duration to date.
///
/// Temporal Equivalent: 3.5.13 `AddDate ( calendar, plainDate, duration [ , options [ , dateAdd ] ] )`
/// 3.5.14 `AddDurationToDate`
///
/// More information:
///
/// - [AO specification](https://tc39.es/proposal-temporal/#sec-temporal-adddurationtodate)
#[inline]
pub(crate) fn add_date(
pub(crate) fn add_duration_to_date(
&self,
duration: &Duration,
overflow: Option<ArithmeticOverflow>,
) -> TemporalResult<Self> {
// 2. If options is not present, set options to undefined.
// 3. If operation is subtract, set duration to CreateNegatedTemporalDuration(duration).
// 4. Let dateDuration be ToDateDurationRecordWithoutTime(duration).
// TODO: Look into why this is fallible, and make some adjustments
let date_duration = duration.to_date_duration_record_without_time()?;
// 5. Let resolvedOptions be ? GetOptionsObject(options).
// 6. Let overflow be ? GetTemporalOverflowOption(resolvedOptions).
let overflow = overflow.unwrap_or(ArithmeticOverflow::Constrain);
// 3. If duration.[[Years]] ≠ 0, or duration.[[Months]] ≠ 0, or duration.[[Weeks]] ≠ 0, then
if duration.date().years != 0 || duration.date().months != 0 || duration.date().weeks != 0 {
// a. If dateAdd is not present, then
// i. Set dateAdd to unused.
// ii. If calendar is an Object, set dateAdd to ? GetMethod(calendar, "dateAdd").
// b. Return ? CalendarDateAdd(calendar, plainDate, duration, options, dateAdd).
return self
.calendar()
.date_add(&self.iso, duration.date(), overflow);
}

// 4. Let overflow be ? ToTemporalOverflow(options).
// 5. Let norm be NormalizeTimeDuration(duration.[[Hours]],
// duration.[[Minutes]], duration.[[Seconds]],
// duration.[[Milliseconds]], duration.[[Microseconds]],
// duration.[[Nanoseconds]]).
// 6. Let days be duration.[[Days]] + BalanceTimeDuration(norm,
// "day").[[Days]].
let days = duration
.days()
.checked_add(
TimeDuration::from_normalized(duration.time().to_normalized(), Unit::Day)?.0,
)
.ok_or(TemporalError::range())?;

// 7. Let result be ? AddISODate(plainDate.[[ISOYear]], plainDate.[[ISOMonth]], plainDate.[[ISODay]], 0, 0, 0, days, overflow).
let result = self
.iso
.add_date_duration(&DateDuration::new(0, 0, 0, days)?, overflow)?;

Self::try_new(
result.year,
result.month,
result.day,
self.calendar().clone(),
)
// 7. Let result be ? CalendarDateAdd(calendar, temporalDate.[[ISODate]], dateDuration, overflow).
// 8. Return ! CreateTemporalDate(result, calendar).
self.calendar()
.date_add(&self.iso, &date_duration, overflow)
}

/// Returns a duration representing the difference between the dates one and two.
Expand Down Expand Up @@ -378,7 +352,7 @@ impl PlainDate {
let result = self.internal_diff_date(other, resolved.largest_unit)?;

// 10. Let duration be ! CreateNormalizedDurationRecord(result.[[Years]], result.[[Months]], result.[[Weeks]], result.[[Days]], ZeroTimeDuration()).
let mut duration = NormalizedDurationRecord::from_date_duration(*result.date())?;
let mut duration = NormalizedDurationRecord::from_date_duration(result.date())?;
// 11. If settings.[[SmallestUnit]] is "day" and settings.[[RoundingIncrement]] = 1, let roundingGranularityIsNoop be true; else let roundingGranularityIsNoop be false.
let rounding_granularity_is_noop =
resolved.smallest_unit == Unit::Day && resolved.increment.get() == 1;
Expand All @@ -399,7 +373,7 @@ impl PlainDate {
resolved,
)?
}
let result = Duration::from_normalized(duration, Unit::Day)?;
let result = Duration::from_internal(duration, Unit::Day)?;
// 13. Return ! CreateTemporalDuration(sign × duration.[[Years]], sign × duration.[[Months]], sign × duration.[[Weeks]], sign × duration.[[Days]], 0, 0, 0, 0, 0, 0).
match op {
DifferenceOperation::Until => Ok(result),
Expand Down Expand Up @@ -593,7 +567,7 @@ impl PlainDate {
duration: &Duration,
overflow: Option<ArithmeticOverflow>,
) -> TemporalResult<Self> {
self.add_date(duration, overflow)
self.add_duration_to_date(duration, overflow)
}

#[inline]
Expand All @@ -603,7 +577,7 @@ impl PlainDate {
duration: &Duration,
overflow: Option<ArithmeticOverflow>,
) -> TemporalResult<Self> {
self.add_date(&duration.negated(), overflow)
self.add_duration_to_date(&duration.negated(), overflow)
}

#[inline]
Expand Down
76 changes: 43 additions & 33 deletions src/builtins/core/datetime.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! This module implements `DateTime` any directly related algorithms.

use super::{
duration::normalized::{NormalizedDurationRecord, NormalizedTimeDuration},
Duration, PartialDate, PartialTime, PlainDate, PlainTime, ZonedDateTime,
duration::normalized::NormalizedDurationRecord, Duration, PartialDate, PartialTime, PlainDate,
PlainTime, ZonedDateTime,
};
use crate::{
builtins::core::{calendar::Calendar, Instant},
Expand All @@ -15,7 +15,7 @@ use crate::{
parsers::{parse_date_time, IxdtfStringBuilder},
primitive::FiniteF64,
provider::{NeverProvider, TimeZoneProvider},
temporal_assert, MonthCode, TemporalError, TemporalResult, TemporalUnwrap, TimeZone,
MonthCode, TemporalError, TemporalResult, TemporalUnwrap, TimeZone,
};
use alloc::string::String;
use core::{cmp::Ordering, str::FromStr};
Expand Down Expand Up @@ -209,33 +209,26 @@ impl PlainDateTime {
overflow: Option<ArithmeticOverflow>,
) -> TemporalResult<Self> {
// SKIP: 1, 2, 3, 4
// 1. If operation is subtract, let sign be -1. Otherwise, let sign be 1.
// 2. Let duration be ? ToTemporalDurationRecord(temporalDurationLike).
// 3. Set options to ? GetOptionsObject(options).
// 4. Let calendarRec be ? CreateCalendarMethodsRecord(dateTime.[[Calendar]], « date-add »).

// 5. Let norm be NormalizeTimeDuration(sign × duration.[[Hours]], sign × duration.[[Minutes]], sign × duration.[[Seconds]], sign × duration.[[Milliseconds]], sign × duration.[[Microseconds]], sign × duration.[[Nanoseconds]]).
let norm = NormalizedTimeDuration::from_time_duration(duration.time());

// TODO: validate Constrain is default with all the recent changes.
// 6. Let result be ? AddDateTime(dateTime.[[ISOYear]], dateTime.[[ISOMonth]], dateTime.[[ISODay]], dateTime.[[ISOHour]], dateTime.[[ISOMinute]], dateTime.[[ISOSecond]], dateTime.[[ISOMillisecond]], dateTime.[[ISOMicrosecond]], dateTime.[[ISONanosecond]], calendarRec, sign × duration.[[Years]], sign × duration.[[Months]], sign × duration.[[Weeks]], sign × duration.[[Days]], norm, options).
let result =
self.iso
.add_date_duration(self.calendar().clone(), duration.date(), norm, overflow)?;

// 7. Assert: IsValidISODate(result.[[Year]], result.[[Month]], result.[[Day]]) is true.
// 8. Assert: IsValidTime(result.[[Hour]], result.[[Minute]], result.[[Second]], result.[[Millisecond]],
// result.[[Microsecond]], result.[[Nanosecond]]) is true.
temporal_assert!(
result.is_within_limits(),
"Assertion failed: the below datetime is not within valid limits:\n{:?}",
result
);

// 9. Return ? CreateTemporalDateTime(result.[[Year]], result.[[Month]], result.[[Day]], result.[[Hour]],
// result.[[Minute]], result.[[Second]], result.[[Millisecond]], result.[[Microsecond]],
// result.[[Nanosecond]], dateTime.[[Calendar]]).
Ok(Self::new_unchecked(result, self.calendar.clone()))
// 5. Let internalDuration be ToInternalDurationRecordWith24HourDays(duration).
let internal_duration =
NormalizedDurationRecord::from_duration_with_24_hour_days(duration)?;
// 6. Let timeResult be AddTime(dateTime.[[ISODateTime]].[[Time]], internalDuration.[[Time]]).
let (days, time_result) = self
.iso
.time
.add(internal_duration.normalized_time_duration());
// 7. Let dateDuration be ? AdjustDateDurationRecord(internalDuration.[[Date]], timeResult.[[Days]]).
let date_duration = internal_duration.date().adjust(days, None, None)?;
// 8. Let addedDate be ? CalendarDateAdd(dateTime.[[Calendar]], dateTime.[[ISODateTime]].[[ISODate]], dateDuration, overflow).
let added_date = self.calendar().date_add(
&self.iso.date,
&date_duration,
overflow.unwrap_or(ArithmeticOverflow::Constrain),
)?;
// 9. Let result be CombineISODateAndTimeRecord(addedDate, timeResult).
let result = IsoDateTime::new(added_date.iso, time_result)?;
// 10. Return ? CreateTemporalDateTime(result, dateTime.[[Calendar]]).
Ok(Self::new_unchecked(result, self.calendar().clone()))
}

/// Difference two `DateTime`s together.
Expand Down Expand Up @@ -268,7 +261,7 @@ impl PlainDateTime {
// Step 10-11.
let norm_record = self.diff_dt_with_rounding(other, options)?;

let result = Duration::from_normalized(norm_record, options.largest_unit)?;
let result = Duration::from_internal(norm_record, options.largest_unit)?;

// Step 12
match op {
Expand Down Expand Up @@ -963,8 +956,8 @@ mod tests {

use crate::{
builtins::core::{
calendar::Calendar, duration::DateDuration, Duration, PartialDate, PartialDateTime,
PartialTime, PlainDateTime,
calendar::Calendar, DateDuration, Duration, PartialDate, PartialDateTime, PartialTime,
PlainDateTime,
},
iso::{IsoDate, IsoDateTime, IsoTime},
options::{
Expand Down Expand Up @@ -1532,4 +1525,21 @@ mod tests {
"pads 4 decimal places to 9"
);
}

#[test]
fn datetime_add() {
use crate::{Duration, PlainDateTime};
use core::str::FromStr;

let dt = PlainDateTime::from_str("2024-01-15T12:00:00").unwrap();

let duration = Duration::from_str("P1M2DT3H4M").unwrap();

// Add duration
let later = dt.add(&duration, None).unwrap();
assert_eq!(later.month(), 2);
assert_eq!(later.day(), 17);
assert_eq!(later.hour(), 15);
assert_eq!(later.minute(), 4);
}
}
Loading
Loading