Skip to content

Commit b8757de

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 3765a52 commit b8757de

Some content is hidden

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

79 files changed

+5522
-3863
lines changed

crates/jiff-static/src/shared/util/escape.rs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use super::utf8;
1717
pub(crate) struct Byte(pub u8);
1818

1919
impl core::fmt::Display for Byte {
20+
#[inline(never)]
2021
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
2122
if self.0 == b' ' {
2223
return write!(f, " ");
@@ -37,6 +38,7 @@ impl core::fmt::Display for Byte {
3738
}
3839

3940
impl core::fmt::Debug for Byte {
41+
#[inline(never)]
4042
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
4143
write!(f, "\"")?;
4244
core::fmt::Display::fmt(self, f)?;
@@ -54,15 +56,16 @@ impl core::fmt::Debug for Byte {
5456
pub(crate) struct Bytes<'a>(pub &'a [u8]);
5557

5658
impl<'a> core::fmt::Display for Bytes<'a> {
59+
#[inline(never)]
5760
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
5861
// This is a sad re-implementation of a similar impl found in bstr.
5962
let mut bytes = self.0;
6063
while let Some(result) = utf8::decode(bytes) {
6164
let ch = match result {
6265
Ok(ch) => ch,
63-
Err(errant_bytes) => {
66+
Err(err) => {
6467
// The decode API guarantees `errant_bytes` is non-empty.
65-
write!(f, r"\x{:02x}", errant_bytes[0])?;
68+
write!(f, r"\x{:02x}", err.as_slice()[0])?;
6669
bytes = &bytes[1..];
6770
continue;
6871
}
@@ -81,6 +84,37 @@ impl<'a> core::fmt::Display for Bytes<'a> {
8184
}
8285

8386
impl<'a> core::fmt::Debug for Bytes<'a> {
87+
#[inline(never)]
88+
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
89+
write!(f, "\"")?;
90+
core::fmt::Display::fmt(self, f)?;
91+
write!(f, "\"")?;
92+
Ok(())
93+
}
94+
}
95+
96+
/// A helper for repeating a single byte utilizing `Byte`.
97+
///
98+
/// This is limited to repeating a byte up to `u8::MAX` times in order
99+
/// to reduce its size overhead. And in practice, Jiff just doesn't
100+
/// need more than this (at time of writing, 2025-11-29).
101+
pub(crate) struct RepeatByte {
102+
pub(crate) byte: u8,
103+
pub(crate) count: u8,
104+
}
105+
106+
impl core::fmt::Display for RepeatByte {
107+
#[inline(never)]
108+
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
109+
for _ in 0..self.count {
110+
write!(f, "{}", Byte(self.byte))?;
111+
}
112+
Ok(())
113+
}
114+
}
115+
116+
impl core::fmt::Debug for RepeatByte {
117+
#[inline(never)]
84118
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
85119
write!(f, "\"")?;
86120
core::fmt::Display::fmt(self, f)?;

crates/jiff-static/src/shared/util/utf8.rs

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,59 @@
11
// auto-generated by: jiff-cli generate shared
22

3+
/// Represents an invalid UTF-8 sequence.
4+
///
5+
/// This is an error returned by `decode`. It is guaranteed to
6+
/// contain 1, 2 or 3 bytes.
7+
pub(crate) struct Utf8Error {
8+
bytes: [u8; 3],
9+
len: u8,
10+
}
11+
12+
impl Utf8Error {
13+
#[cold]
14+
#[inline(never)]
15+
fn new(original_bytes: &[u8], err: core::str::Utf8Error) -> Utf8Error {
16+
let len = err.error_len().unwrap_or_else(|| original_bytes.len());
17+
// OK because the biggest invalid UTF-8
18+
// sequence possible is 3.
19+
debug_assert!(1 <= len && len <= 3);
20+
let mut bytes = [0; 3];
21+
bytes[..len].copy_from_slice(&original_bytes[..len]);
22+
Utf8Error {
23+
bytes,
24+
// OK because the biggest invalid UTF-8
25+
// sequence possible is 3.
26+
len: u8::try_from(len).unwrap(),
27+
}
28+
}
29+
30+
/// Returns the slice of invalid UTF-8 bytes.
31+
///
32+
/// The slice returned is guaranteed to have length equivalent
33+
/// to `Utf8Error::len`.
34+
pub(crate) fn as_slice(&self) -> &[u8] {
35+
&self.bytes[..self.len()]
36+
}
37+
38+
/// Returns the length of the invalid UTF-8 sequence found.
39+
///
40+
/// This is guaranteed to be 1, 2 or 3.
41+
pub(crate) fn len(&self) -> usize {
42+
usize::from(self.len)
43+
}
44+
}
45+
46+
impl core::fmt::Display for Utf8Error {
47+
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
48+
write!(
49+
f,
50+
"found invalid UTF-8 byte {errant_bytes:?} in format \
51+
string (format strings must be valid UTF-8)",
52+
errant_bytes = crate::shared::util::escape::Bytes(self.as_slice()),
53+
)
54+
}
55+
}
56+
357
/// Decodes the next UTF-8 encoded codepoint from the given byte slice.
458
///
559
/// If no valid encoding of a codepoint exists at the beginning of the
@@ -15,22 +69,20 @@
1569
/// *WARNING*: This is not designed for performance. If you're looking for
1670
/// a fast UTF-8 decoder, this is not it. If you feel like you need one in
1771
/// this crate, then please file an issue and discuss your use case.
18-
pub(crate) fn decode(bytes: &[u8]) -> Option<Result<char, &[u8]>> {
72+
pub(crate) fn decode(bytes: &[u8]) -> Option<Result<char, Utf8Error>> {
1973
if bytes.is_empty() {
2074
return None;
2175
}
2276
let string = match core::str::from_utf8(&bytes[..bytes.len().min(4)]) {
2377
Ok(s) => s,
2478
Err(ref err) if err.valid_up_to() > 0 => {
79+
// OK because we just verified we have at least some
80+
// valid UTF-8.
2581
core::str::from_utf8(&bytes[..err.valid_up_to()]).unwrap()
2682
}
2783
// In this case, we want to return 1-3 bytes that make up a prefix of
2884
// a potentially valid codepoint.
29-
Err(err) => {
30-
return Some(Err(
31-
&bytes[..err.error_len().unwrap_or_else(|| bytes.len())]
32-
))
33-
}
85+
Err(err) => return Some(Err(Utf8Error::new(bytes, err))),
3486
};
3587
// OK because we guaranteed above that `string`
3688
// must be non-empty. And thus, `str::chars` must

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

0 commit comments

Comments
 (0)