Skip to content

Commit 1d44e8d

Browse files
committed
error: switch everything over to structured errors
This was an incredibly tedious and tortuous refactor. But this removes almost all of the "create ad hoc stringly-typed errors everywhere." This partially makes progress toward #418, but my initial impetus for doing this was to see if I could reduce binary size and improve compilation times. My general target was to see if I could reduce total LLVM lines. I tested this with [Biff] using this command in the root of the Biff repo: ``` cargo llvm-lines --profile release-lto ``` Before this change, Biff had 768,596 LLVM lines. With this change, it has 757,331 lines. So... an improvement, but a very modest one. What about compilation times? This does seem to translate to---also a modest---improvement. For compiling release builds of Biff. Before: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b -r' Benchmark 1: cargo b -r Time (mean ± σ): 7.776 s ± 0.052 s [User: 65.876 s, System: 2.621 s] Range (min … max): 7.690 s … 7.862 s 10 runs ``` After: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b -r' Benchmark 1: cargo b -r Time (mean ± σ): 7.591 s ± 0.067 s [User: 65.686 s, System: 2.564 s] Range (min … max): 7.504 s … 7.689 s 10 runs ``` What about dev builds? Before: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b' Benchmark 1: cargo b Time (mean ± σ): 4.074 s ± 0.022 s [User: 14.493 s, System: 1.818 s] Range (min … max): 4.037 s … 4.099 s 10 runs ``` After: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b' Benchmark 1: cargo b Time (mean ± σ): 4.541 s ± 0.027 s [User: 15.385 s, System: 2.081 s] Range (min … max): 4.503 s … 4.591 s 10 runs ``` Well... that's disappointing. A modest improvement to release builds, but a fairly large regression in dev builds. Maybe it's because of the additional hand-written impls for new structured error types? Bah. And binary size? Normal release builds (not LTO) of Biff that were stripped were 4,431,456 bytes before this change and 4,392,064 after. Hopefully this will unlock other improvements to justify doing this. Note also that this slims down a number of error messages. [Biff]: https://github.com/BurntSushi/biff
1 parent 0a5c7e9 commit 1d44e8d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

76 files changed

+5418
-3840
lines changed

src/civil/date.rs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use core::time::Duration as UnsignedDuration;
33
use crate::{
44
civil::{DateTime, Era, ISOWeekDate, Time, Weekday},
55
duration::{Duration, SDuration},
6-
error::{err, Error, ErrorContext},
6+
error::{civil::Error as E, Error, ErrorContext},
77
fmt::{
88
self,
99
temporal::{DEFAULT_DATETIME_PARSER, DEFAULT_DATETIME_PRINTER},
@@ -1057,7 +1057,7 @@ impl Date {
10571057

10581058
let nth = t::SpanWeeks::try_new("nth weekday", nth)?;
10591059
if nth == C(0) {
1060-
Err(err!("nth weekday cannot be `0`"))
1060+
Err(Error::from(E::NthWeekdayNonZero))
10611061
} else if nth > C(0) {
10621062
let nth = nth.max(C(1));
10631063
let weekday_diff = weekday.since_ranged(self.weekday().next());
@@ -1515,14 +1515,8 @@ impl Date {
15151515
-1 => self.yesterday(),
15161516
1 => self.tomorrow(),
15171517
days => {
1518-
let days = UnixEpochDay::try_new("days", days).with_context(
1519-
|| {
1520-
err!(
1521-
"{days} computed from duration {duration:?} \
1522-
overflows Jiff's datetime limits",
1523-
)
1524-
},
1525-
)?;
1518+
let days = UnixEpochDay::try_new("days", days)
1519+
.context(E::OverflowDaysDuration)?;
15261520
let days =
15271521
self.to_unix_epoch_day().try_checked_add("days", days)?;
15281522
Ok(Date::from_unix_epoch_day(days))
@@ -2941,11 +2935,9 @@ impl DateDifference {
29412935
//
29422936
// NOTE: I take the above back. It's actually possible for the
29432937
// months component to overflow when largest=month.
2944-
return Err(err!(
2945-
"rounding the span between two dates must use days \
2946-
or bigger for its units, but found {units}",
2947-
units = largest.plural(),
2948-
));
2938+
return Err(Error::from(E::RoundMustUseDaysOrBigger {
2939+
unit: largest,
2940+
}));
29492941
}
29502942
if largest <= Unit::Week {
29512943
let mut weeks = t::SpanWeeks::rfrom(C(0));

src/civil/datetime.rs

Lines changed: 26 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
datetime, Date, DateWith, Era, ISOWeekDate, Time, TimeWith, Weekday,
66
},
77
duration::{Duration, SDuration},
8-
error::{err, Error, ErrorContext},
8+
error::{civil::Error as E, Error, ErrorContext},
99
fmt::{
1010
self,
1111
temporal::{self, DEFAULT_DATETIME_PARSER},
@@ -1695,25 +1695,18 @@ impl DateTime {
16951695
{
16961696
(true, true) => Ok(self),
16971697
(false, true) => {
1698-
let new_date =
1699-
old_date.checked_add(span).with_context(|| {
1700-
err!("failed to add {span} to {old_date}")
1701-
})?;
1698+
let new_date = old_date
1699+
.checked_add(span)
1700+
.context(E::FailedAddSpanDate)?;
17021701
Ok(DateTime::from_parts(new_date, old_time))
17031702
}
17041703
(true, false) => {
1705-
let (new_time, leftovers) =
1706-
old_time.overflowing_add(span).with_context(|| {
1707-
err!("failed to add {span} to {old_time}")
1708-
})?;
1709-
let new_date =
1710-
old_date.checked_add(leftovers).with_context(|| {
1711-
err!(
1712-
"failed to add overflowing span, {leftovers}, \
1713-
from adding {span} to {old_time}, \
1714-
to {old_date}",
1715-
)
1716-
})?;
1704+
let (new_time, leftovers) = old_time
1705+
.overflowing_add(span)
1706+
.context(E::FailedAddSpanTime)?;
1707+
let new_date = old_date
1708+
.checked_add(leftovers)
1709+
.context(E::FailedAddSpanOverflowing)?;
17171710
Ok(DateTime::from_parts(new_date, new_time))
17181711
}
17191712
(false, false) => self.checked_add_span_general(&span),
@@ -1727,20 +1720,14 @@ impl DateTime {
17271720
let span_date = span.without_lower(Unit::Day);
17281721
let span_time = span.only_lower(Unit::Day);
17291722

1730-
let (new_time, leftovers) =
1731-
old_time.overflowing_add(span_time).with_context(|| {
1732-
err!("failed to add {span_time} to {old_time}")
1733-
})?;
1734-
let new_date = old_date.checked_add(span_date).with_context(|| {
1735-
err!("failed to add {span_date} to {old_date}")
1736-
})?;
1737-
let new_date = new_date.checked_add(leftovers).with_context(|| {
1738-
err!(
1739-
"failed to add overflowing span, {leftovers}, \
1740-
from adding {span_time} to {old_time}, \
1741-
to {new_date}",
1742-
)
1743-
})?;
1723+
let (new_time, leftovers) = old_time
1724+
.overflowing_add(span_time)
1725+
.context(E::FailedAddSpanTime)?;
1726+
let new_date =
1727+
old_date.checked_add(span_date).context(E::FailedAddSpanDate)?;
1728+
let new_date = new_date
1729+
.checked_add(leftovers)
1730+
.context(E::FailedAddSpanOverflowing)?;
17441731
Ok(DateTime::from_parts(new_date, new_time))
17451732
}
17461733

@@ -1751,13 +1738,9 @@ impl DateTime {
17511738
) -> Result<DateTime, Error> {
17521739
let (date, time) = (self.date(), self.time());
17531740
let (new_time, leftovers) = time.overflowing_add_duration(duration)?;
1754-
let new_date = date.checked_add(leftovers).with_context(|| {
1755-
err!(
1756-
"failed to add overflowing signed duration, {leftovers:?}, \
1757-
from adding {duration:?} to {time},
1758-
to {date}",
1759-
)
1760-
})?;
1741+
let new_date = date
1742+
.checked_add(leftovers)
1743+
.context(E::FailedAddDurationOverflowing)?;
17611744
Ok(DateTime::from_parts(new_date, new_time))
17621745
}
17631746

@@ -3552,9 +3535,10 @@ impl DateTimeRound {
35523535
// it for good reasons.
35533536
match self.smallest {
35543537
Unit::Year | Unit::Month | Unit::Week => {
3555-
return Err(err!(
3556-
"rounding datetimes does not support {unit}",
3557-
unit = self.smallest.plural()
3538+
return Err(Error::from(
3539+
crate::error::util::RoundingIncrementError::Unsupported {
3540+
unit: self.smallest,
3541+
},
35583542
));
35593543
}
35603544
// We don't do any rounding in this case, so just bail now.
@@ -3592,9 +3576,7 @@ impl DateTimeRound {
35923576
// supported datetimes.
35933577
let end = start
35943578
.checked_add(Span::new().days_ranged(days_len))
3595-
.with_context(|| {
3596-
err!("adding {days_len} days to {start} failed")
3597-
})?;
3579+
.context(E::FailedAddDays)?;
35983580
Ok(DateTime::from_parts(end, time))
35993581
}
36003582

src/civil/iso_week_date.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
22
civil::{Date, DateTime, Weekday},
3-
error::{err, Error},
3+
error::{civil::Error as E, Error},
44
fmt::temporal::{DEFAULT_DATETIME_PARSER, DEFAULT_DATETIME_PRINTER},
55
util::{
66
rangeint::RInto,
@@ -711,9 +711,7 @@ impl ISOWeekDate {
711711
debug_assert_eq!(t::Year::MIN, ISOYear::MIN);
712712
debug_assert_eq!(t::Year::MAX, ISOYear::MAX);
713713
if week == C(53) && !is_long_year(year) {
714-
return Err(err!(
715-
"ISO week number `{week}` is invalid for year `{year}`"
716-
));
714+
return Err(Error::from(E::InvalidISOWeekNumber));
717715
}
718716
// And also, the maximum Date constrains what we can utter with
719717
// ISOWeekDate so that we can preserve infallible conversions between

src/civil/time.rs

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use core::time::Duration as UnsignedDuration;
33
use crate::{
44
civil::{Date, DateTime},
55
duration::{Duration, SDuration},
6-
error::{err, Error, ErrorContext},
6+
error::{civil::Error as E, Error, ErrorContext},
77
fmt::{
88
self,
99
temporal::{self, DEFAULT_DATETIME_PARSER},
@@ -950,23 +950,14 @@ impl Time {
950950
self,
951951
duration: SignedDuration,
952952
) -> Result<Time, Error> {
953-
let original = duration;
954953
let start = t::NoUnits128::rfrom(self.to_nanosecond());
955954
let duration = t::NoUnits128::new_unchecked(duration.as_nanos());
956955
// This can never fail because the maximum duration fits into a
957956
// 96-bit integer, and adding any 96-bit integer to any 64-bit
958957
// integer can never overflow a 128-bit integer.
959958
let end = start.try_checked_add("nanoseconds", duration).unwrap();
960959
let end = CivilDayNanosecond::try_rfrom("nanoseconds", end)
961-
.with_context(|| {
962-
err!(
963-
"adding signed duration {duration:?}, equal to
964-
{nanos} nanoseconds, to {time} overflowed",
965-
duration = original,
966-
nanos = original.as_nanos(),
967-
time = self,
968-
)
969-
})?;
960+
.context(E::OverflowTimeNanoseconds)?;
970961
Ok(Time::from_nanosecond(end))
971962
}
972963

@@ -2603,11 +2594,9 @@ impl TimeDifference {
26032594
}
26042595
let largest = self.round.get_largest().unwrap_or(Unit::Hour);
26052596
if largest > Unit::Hour {
2606-
return Err(err!(
2607-
"rounding the span between two times must use hours \
2608-
or smaller for its units, but found {units}",
2609-
units = largest.plural(),
2610-
));
2597+
return Err(Error::from(E::RoundMustUseHoursOrSmaller {
2598+
unit: largest,
2599+
}));
26112600
}
26122601
let start = t1.to_nanosecond();
26132602
let end = t2.to_nanosecond();
@@ -3012,22 +3001,13 @@ impl TimeWith {
30123001
None => self.original.subsec_nanosecond_ranged(),
30133002
Some(subsec_nanosecond) => {
30143003
if self.millisecond.is_some() {
3015-
return Err(err!(
3016-
"cannot set both TimeWith::millisecond \
3017-
and TimeWith::subsec_nanosecond",
3018-
));
3004+
return Err(Error::from(E::IllegalTimeWithMillisecond));
30193005
}
30203006
if self.microsecond.is_some() {
3021-
return Err(err!(
3022-
"cannot set both TimeWith::microsecond \
3023-
and TimeWith::subsec_nanosecond",
3024-
));
3007+
return Err(Error::from(E::IllegalTimeWithMicrosecond));
30253008
}
30263009
if self.nanosecond.is_some() {
3027-
return Err(err!(
3028-
"cannot set both TimeWith::nanosecond \
3029-
and TimeWith::subsec_nanosecond",
3030-
));
3010+
return Err(Error::from(E::IllegalTimeWithNanosecond));
30313011
}
30323012
SubsecNanosecond::try_new(
30333013
"subsec_nanosecond",

src/duration.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use core::time::Duration as UnsignedDuration;
22

33
use crate::{
4-
error::{err, ErrorContext},
4+
error::{duration::Error as E, ErrorContext},
55
Error, SignedDuration, Span,
66
};
77

@@ -24,12 +24,8 @@ impl Duration {
2424
Duration::Span(span) => Ok(SDuration::Span(span)),
2525
Duration::Signed(sdur) => Ok(SDuration::Absolute(sdur)),
2626
Duration::Unsigned(udur) => {
27-
let sdur =
28-
SignedDuration::try_from(udur).with_context(|| {
29-
err!(
30-
"unsigned duration {udur:?} exceeds Jiff's limits"
31-
)
32-
})?;
27+
let sdur = SignedDuration::try_from(udur)
28+
.context(E::RangeUnsignedDuration)?;
3329
Ok(SDuration::Absolute(sdur))
3430
}
3531
}
@@ -91,9 +87,8 @@ impl Duration {
9187
// Otherwise, this is the only failure point in this entire
9288
// routine. And specifically, we fail here in precisely
9389
// the cases where `udur.as_secs() > |i64::MIN|`.
94-
-SignedDuration::try_from(udur).with_context(|| {
95-
err!("failed to negate unsigned duration {udur:?}")
96-
})?
90+
-SignedDuration::try_from(udur)
91+
.context(E::FailedNegateUnsignedDuration)?
9792
};
9893
Ok(Duration::Signed(sdur))
9994
}

src/error/civil.rs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
use crate::{error, Unit};
2+
3+
#[derive(Clone, Debug)]
4+
pub(crate) enum Error {
5+
FailedAddDays,
6+
FailedAddDurationOverflowing,
7+
FailedAddSpanDate,
8+
FailedAddSpanOverflowing,
9+
FailedAddSpanTime,
10+
IllegalTimeWithMicrosecond,
11+
IllegalTimeWithMillisecond,
12+
IllegalTimeWithNanosecond,
13+
InvalidISOWeekNumber,
14+
OverflowDaysDuration,
15+
OverflowTimeNanoseconds,
16+
NthWeekdayNonZero,
17+
RoundMustUseDaysOrBigger { unit: Unit },
18+
RoundMustUseHoursOrSmaller { unit: Unit },
19+
}
20+
21+
impl From<Error> for error::Error {
22+
#[cold]
23+
#[inline(never)]
24+
fn from(err: Error) -> error::Error {
25+
error::ErrorKind::Civil(err).into()
26+
}
27+
}
28+
29+
impl error::IntoError for Error {
30+
fn into_error(self) -> error::Error {
31+
self.into()
32+
}
33+
}
34+
35+
impl core::fmt::Display for Error {
36+
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
37+
use self::Error::*;
38+
39+
match *self {
40+
FailedAddDays => f.write_str("failed to add days to date"),
41+
FailedAddDurationOverflowing => {
42+
f.write_str("failed to add overflowing duration")
43+
}
44+
FailedAddSpanDate => f.write_str("failed to add span to date"),
45+
FailedAddSpanOverflowing => {
46+
f.write_str("failed to add overflowing span")
47+
}
48+
FailedAddSpanTime => f.write_str("failed to add span to time"),
49+
IllegalTimeWithMicrosecond => f.write_str(
50+
"cannot set both `TimeWith::microsecond` \
51+
and `TimeWith::subsec_nanosecond`",
52+
),
53+
IllegalTimeWithMillisecond => f.write_str(
54+
"cannot set both `TimeWith::millisecond` \
55+
and `TimeWith::subsec_nanosecond`",
56+
),
57+
IllegalTimeWithNanosecond => f.write_str(
58+
"cannot set both `TimeWith::nanosecond` \
59+
and `TimeWith::subsec_nanosecond`",
60+
),
61+
InvalidISOWeekNumber => {
62+
f.write_str("ISO week number is invalid for given year")
63+
}
64+
OverflowDaysDuration => f.write_str(
65+
"number of days derived from duration exceed's \
66+
Jiff's datetime limits",
67+
),
68+
OverflowTimeNanoseconds => {
69+
f.write_str("adding duration to time overflowed")
70+
}
71+
NthWeekdayNonZero => f.write_str("nth weekday cannot be `0`"),
72+
RoundMustUseDaysOrBigger { unit } => write!(
73+
f,
74+
"rounding the span between two dates must use days \
75+
or bigger for its units, but found {unit}",
76+
unit = unit.plural(),
77+
),
78+
RoundMustUseHoursOrSmaller { unit } => write!(
79+
f,
80+
"rounding the span between two times must use hours \
81+
or smaller for its units, but found {unit}",
82+
unit = unit.plural(),
83+
),
84+
}
85+
}
86+
}

0 commit comments

Comments
 (0)