Skip to content

Commit 53fc1fc

Browse files
authored
Bug fixes to address test failures + removing unused API (#162)
This PR fixes both an assertion bug from caused by `IncrementRounder`'s as_positive path. Since this was the only use case in the specification that seemed to be using the "as_positive" path, I completely removed the methods. This also fixes some other bugs caused from using non_euclidean operations in `IsoDateTime::from_epoch_nanos`
1 parent 46dcd52 commit 53fc1fc

File tree

7 files changed

+123
-171
lines changed

7 files changed

+123
-171
lines changed

Cargo.lock

Lines changed: 0 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,10 @@ exclude.workspace = true
4949
[dependencies]
5050
tinystr.workspace = true
5151
icu_calendar = { workspace = true, features = ["compiled_data"] }
52-
rustc-hash = { workspace = true, features = ["std"] }
53-
bitflags.workspace = true
5452
num-traits.workspace = true
5553
ixdtf = { workspace = true, features = ["duration"]}
5654
iana-time-zone.workspace = true
55+
writeable = "0.6.0"
5756

5857
# log feature
5958
log = { workspace = true, optional = true }
@@ -65,7 +64,6 @@ combine = { workspace = true, optional = true }
6564

6665
# System time feature
6766
web-time = { workspace = true, optional = true }
68-
writeable = "0.6.0"
6967

7068
[features]
7169
default = ["now"]

src/components/duration/normalized.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ impl NormalizedTimeDuration {
145145
// a. Let fractionalDays be days + DivideNormalizedTimeDuration(norm, nsPerDay).
146146
let fractional_days = days.checked_add(&FiniteF64(self.as_fractional_days()))?;
147147
// b. Set days to RoundNumberToIncrement(fractionalDays, increment, roundingMode).
148-
let days = IncrementRounder::from_potentially_negative_parts(
148+
let days = IncrementRounder::from_signed_num(
149149
fractional_days.0,
150150
options.increment.as_extended_increment(),
151151
)?
@@ -204,8 +204,7 @@ impl NormalizedTimeDuration {
204204
increment: NonZeroU128,
205205
mode: TemporalRoundingMode,
206206
) -> TemporalResult<Self> {
207-
let rounded = IncrementRounder::<i128>::from_potentially_negative_parts(self.0, increment)?
208-
.round(mode);
207+
let rounded = IncrementRounder::<i128>::from_signed_num(self.0, increment)?.round(mode);
209208
if rounded.abs() > MAX_TIME_DURATION {
210209
return Err(TemporalError::range()
211210
.with_message("normalizedTimeDuration exceeds maxTimeDuration."));
@@ -311,7 +310,7 @@ impl NormalizedDurationRecord {
311310
// 1. If unit is "year", then
312311
TemporalUnit::Year => {
313312
// a. Let years be RoundNumberToIncrement(duration.[[Years]], increment, "trunc").
314-
let years = IncrementRounder::from_potentially_negative_parts(
313+
let years = IncrementRounder::from_signed_num(
315314
self.date().years.0,
316315
options.increment.as_extended_increment(),
317316
)?
@@ -343,7 +342,7 @@ impl NormalizedDurationRecord {
343342
// 2. Else if unit is "month", then
344343
TemporalUnit::Month => {
345344
// a. Let months be RoundNumberToIncrement(duration.[[Months]], increment, "trunc").
346-
let months = IncrementRounder::from_potentially_negative_parts(
345+
let months = IncrementRounder::from_signed_num(
347346
self.date().months.0,
348347
options.increment.as_extended_increment(),
349348
)?
@@ -416,7 +415,7 @@ impl NormalizedDurationRecord {
416415
weeks_start.internal_diff_date(&weeks_end, TemporalUnit::Week)?;
417416

418417
// h. Let weeks be RoundNumberToIncrement(duration.[[Weeks]] + untilResult.[[Weeks]], increment, "trunc").
419-
let weeks = IncrementRounder::from_potentially_negative_parts(
418+
let weeks = IncrementRounder::from_signed_num(
420419
self.date().weeks.checked_add(&until_result.weeks())?.0,
421420
options.increment.as_extended_increment(),
422421
)?
@@ -450,7 +449,7 @@ impl NormalizedDurationRecord {
450449
// 4. Else,
451450
// a. Assert: unit is "day".
452451
// b. Let days be RoundNumberToIncrement(duration.[[Days]], increment, "trunc").
453-
let days = IncrementRounder::from_potentially_negative_parts(
452+
let days = IncrementRounder::from_signed_num(
454453
self.date().days.0,
455454
options.increment.as_extended_increment(),
456455
)?
@@ -550,11 +549,9 @@ impl NormalizedDurationRecord {
550549
// This division can be implemented as if constructing Normalized Time Duration Records for the denominator
551550
// and numerator of total and performing one division operation with a floating-point result.
552551
// 15. Let roundedUnit be ApplyUnsignedRoundingMode(total, r1, r2, unsignedRoundingMode).
553-
let rounded_unit = IncrementRounder::from_potentially_negative_parts(
554-
total,
555-
options.increment.as_extended_increment(),
556-
)?
557-
.round(options.rounding_mode);
552+
let rounded_unit =
553+
IncrementRounder::from_signed_num(total, options.increment.as_extended_increment())?
554+
.round(options.rounding_mode);
558555

559556
// 16. If roundedUnit - total < 0, let roundedSign be -1; else let roundedSign be 1.
560557
// 19. Return Duration Nudge Result Record { [[Duration]]: resultDuration, [[Total]]: total, [[NudgedEpochNs]]: nudgedEpochNs, [[DidExpandCalendarUnit]]: didExpandCalendarUnit }.

src/components/instant.rs

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,10 @@ impl Instant {
175175
return Err(TemporalError::range().with_message("Increment exceeded a valid range."));
176176
};
177177

178-
let rounded = IncrementRounder::<i128>::from_positive_parts(self.as_i128(), increment)?
179-
.round_as_positive(resolved_options.rounding_mode);
178+
let rounded = IncrementRounder::<i128>::from_signed_num(self.as_i128(), increment)?
179+
.round(resolved_options.rounding_mode);
180180

181-
Ok(rounded as i128)
181+
Ok(rounded)
182182
}
183183

184184
// Utility for converting `Instant` to `i128`.
@@ -357,6 +357,7 @@ impl FromStr for Instant {
357357

358358
#[cfg(test)]
359359
mod tests {
360+
360361
use crate::{
361362
components::{duration::TimeDuration, Instant},
362363
options::{DifferenceSettings, TemporalRoundingMode, TemporalUnit},
@@ -559,4 +560,82 @@ mod tests {
559560
(-376435.0, -23.0, -8.0, -148.0, -529.0, -500.0),
560561
);
561562
}
563+
564+
// /test/built-ins/Temporal/Instant/prototype/add/cross-epoch.js
565+
#[cfg(feature = "tzdb")]
566+
#[test]
567+
fn instant_add_across_epoch() {
568+
use crate::{
569+
options::ToStringRoundingOptions, partial::PartialDuration, tzdb::FsTzdbProvider,
570+
Duration,
571+
};
572+
use core::str::FromStr;
573+
574+
let instant = Instant::from_str("1969-12-25T12:23:45.678901234Z").unwrap();
575+
let one = instant
576+
.subtract(
577+
Duration::from_partial_duration(PartialDuration {
578+
hours: Some(240.into()),
579+
nanoseconds: Some(800.into()),
580+
..Default::default()
581+
})
582+
.unwrap(),
583+
)
584+
.unwrap();
585+
let two = instant
586+
.add(
587+
Duration::from_partial_duration(PartialDuration {
588+
hours: Some(240.into()),
589+
nanoseconds: Some(800.into()),
590+
..Default::default()
591+
})
592+
.unwrap(),
593+
)
594+
.unwrap();
595+
let three = two
596+
.subtract(
597+
Duration::from_partial_duration(PartialDuration {
598+
hours: Some(480.into()),
599+
nanoseconds: Some(1600.into()),
600+
..Default::default()
601+
})
602+
.unwrap(),
603+
)
604+
.unwrap();
605+
let four = one
606+
.add(
607+
Duration::from_partial_duration(PartialDuration {
608+
hours: Some(480.into()),
609+
nanoseconds: Some(1600.into()),
610+
..Default::default()
611+
})
612+
.unwrap(),
613+
)
614+
.unwrap();
615+
616+
let one_comp = Instant::from_str("1969-12-15T12:23:45.678900434Z").unwrap();
617+
let two_comp = Instant::from_str("1970-01-04T12:23:45.678902034Z").unwrap();
618+
619+
// Assert the comparisons all hold.
620+
assert_eq!(one, one_comp);
621+
assert_eq!(two, two_comp);
622+
assert_eq!(three, one);
623+
assert_eq!(four, two);
624+
625+
// Assert the to_string is valid.
626+
let provider = &FsTzdbProvider::default();
627+
let inst_string = instant
628+
.to_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
629+
.unwrap();
630+
let one_string = one
631+
.to_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
632+
.unwrap();
633+
let two_string = two
634+
.to_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
635+
.unwrap();
636+
637+
assert_eq!(&inst_string, "1969-12-25T12:23:45.678901234Z");
638+
assert_eq!(&one_string, "1969-12-15T12:23:45.678900434Z");
639+
assert_eq!(&two_string, "1970-01-04T12:23:45.678902034Z");
640+
}
562641
}

src/components/zoneddatetime.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,7 +1116,7 @@ pub(crate) fn interpret_isodatetime_offset(
11161116
// c. If matchBehaviour is match-minutes, then
11171117
if match_minutes {
11181118
// i. Let roundedCandidateNanoseconds be RoundNumberToIncrement(candidateOffset, 60 × 10**9, half-expand).
1119-
let rounded_candidate = IncrementRounder::from_potentially_negative_parts(
1119+
let rounded_candidate = IncrementRounder::from_signed_num(
11201120
candidate_offset,
11211121
NonZeroU128::new(60_000_000_000).expect("cannot be zero"), // TODO: Add back const { } after MSRV can be bumped
11221122
)?
@@ -1159,7 +1159,7 @@ pub(crate) fn nanoseconds_to_formattable_offset_minutes(
11591159
nanoseconds: i128,
11601160
) -> TemporalResult<(Sign, u8, u8)> {
11611161
// Per 11.1.7 this should be rounding
1162-
let nanoseconds = IncrementRounder::from_potentially_negative_parts(nanoseconds, unsafe {
1162+
let nanoseconds = IncrementRounder::from_signed_num(nanoseconds, unsafe {
11631163
NonZeroU128::new_unchecked(NS_PER_MINUTE as u128)
11641164
})?
11651165
.round(TemporalRoundingMode::HalfExpand);

src/iso.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ impl IsoDateTime {
8585
})?;
8686

8787
// 2. Let remainderNs be epochNanoseconds modulo 10^6.
88-
let remainder_nanos = mathematical_nanos % 1_000_000;
88+
let remainder_nanos = mathematical_nanos.rem_euclid(1_000_000);
8989

9090
// 3. Let epochMilliseconds be 𝔽((epochNanoseconds - remainderNs) / 10^6).
9191
let epoch_millis = (mathematical_nanos - remainder_nanos) / 1_000_000;
@@ -95,20 +95,20 @@ impl IsoDateTime {
9595
let day = utils::epoch_time_to_date(epoch_millis as f64);
9696

9797
// 7. Let hour be ℝ(! HourFromTime(epochMilliseconds)).
98-
let hour = (epoch_millis / 3_600_000) % 24;
99-
// 8. Let minute be ℝ(! MinFromTime(epochMilliseconds)).
100-
let minute = (epoch_millis / 60_000) % 60;
98+
let hour = epoch_millis.div_euclid(3_600_000).rem_euclid(24);
99+
// 8. Let minute be ℝ(! MinFromTime(epochMilliserhs)conds)).
100+
let minute = epoch_millis.div_euclid(60_000).rem_euclid(60);
101101
// 9. Let second be ℝ(! SecFromTime(epochMilliseconds)).
102-
let second = (epoch_millis / 1000) % 60;
102+
let second = epoch_millis.div_euclid(1000).rem_euclid(60);
103103
// 10. Let millisecond be ℝ(! msFromTime(epochMilliseconds)).
104-
let millis = (epoch_millis % 1000) % 1000;
104+
let millis = epoch_millis.rem_euclid(1000);
105105

106106
// 11. Let microsecond be floor(remainderNs / 1000).
107-
let micros = remainder_nanos / 1000;
107+
let micros = remainder_nanos.div_euclid(1000);
108108
// 12. Assert: microsecond < 1000.
109109
temporal_assert!(micros < 1000);
110110
// 13. Let nanosecond be remainderNs modulo 1000.
111-
let nanos = remainder_nanos % 1000;
111+
let nanos = remainder_nanos.rem_euclid(1000);
112112

113113
Ok(Self::balance(
114114
year,
@@ -787,10 +787,9 @@ impl IsoTime {
787787
.ok_or(TemporalError::range().with_message("increment exceeded valid range."))?;
788788

789789
// 8. Let result be RoundNumberToIncrement(quantity, increment × unitLength, roundingMode) / unitLength.
790-
let result =
791-
IncrementRounder::<i128>::from_potentially_negative_parts(quantity, increment)?
792-
.round(resolved_options.rounding_mode)
793-
/ length.get() as i128;
790+
let result = IncrementRounder::<i128>::from_signed_num(quantity, increment)?
791+
.round(resolved_options.rounding_mode)
792+
/ length.get() as i128;
794793

795794
let result_i64 = i64::from_i128(result)
796795
.ok_or(TemporalError::range().with_message("round result valid range."))?;

0 commit comments

Comments
 (0)