Skip to content

Commit 7ad7a2d

Browse files
jhprattjhpratt
committed
Simplify ComponentRange error type
This significantly reduces the size of multiple error types at minimal harm to diagnostics
1 parent f5cdb15 commit 7ad7a2d

File tree

16 files changed

+104
-239
lines changed

16 files changed

+104
-239
lines changed

‎time/src/date.rs‎

Lines changed: 7 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,7 @@ impl Date {
156156
29..=31 if day <= days_in_month_leap(month as u8, is_leap_year) => hint::cold_path(),
157157
_ => {
158158
hint::cold_path();
159-
return Err(error::ComponentRange {
160-
name: "day",
161-
minimum: 1,
162-
maximum: days_in_month_leap(month as u8, is_leap_year) as i64,
163-
value: day as i64,
164-
conditional_message: Some("for the given month and year"),
165-
});
159+
return Err(error::ComponentRange::conditional("day"));
166160
}
167161
}
168162

@@ -198,13 +192,7 @@ impl Date {
198192
366 if is_leap_year => hint::cold_path(),
199193
_ => {
200194
hint::cold_path();
201-
return Err(error::ComponentRange {
202-
name: "ordinal",
203-
minimum: 1,
204-
maximum: if is_leap_year { 366 } else { 365 },
205-
value: ordinal as i64,
206-
conditional_message: Some("for the given year"),
207-
});
195+
return Err(error::ComponentRange::conditional("ordinal"));
208196
}
209197
}
210198

@@ -236,13 +224,7 @@ impl Date {
236224
53 if week <= weeks_in_year(year) => hint::cold_path(),
237225
_ => {
238226
hint::cold_path();
239-
return Err(error::ComponentRange {
240-
name: "week",
241-
minimum: 1,
242-
maximum: weeks_in_year(year) as i64,
243-
value: week as i64,
244-
conditional_message: Some("for the given year"),
245-
});
227+
return Err(error::ComponentRange::conditional("week"));
246228
}
247229
}
248230

@@ -1148,13 +1130,7 @@ impl Date {
11481130
})
11491131
}
11501132
// February 29 does not exist in common years.
1151-
(true, false) if ordinal == 60 => Err(error::ComponentRange {
1152-
name: "day",
1153-
value: 29,
1154-
minimum: 1,
1155-
maximum: 28,
1156-
conditional_message: Some("for the given month and year"),
1157-
}),
1133+
(true, false) if ordinal == 60 => Err(error::ComponentRange::conditional("day")),
11581134
// We're going from a common year to a leap year. Shift dates in March and later by
11591135
// one day.
11601136
// Safety: `ordinal` is not zero and `is_leap_year` is correct.
@@ -1205,13 +1181,7 @@ impl Date {
12051181
29..=31 if day <= days_in_month_leap(month as u8, is_leap_year) => hint::cold_path(),
12061182
_ => {
12071183
hint::cold_path();
1208-
return Err(error::ComponentRange {
1209-
name: "day",
1210-
minimum: 1,
1211-
maximum: days_in_month_leap(month as u8, is_leap_year) as i64,
1212-
value: day as i64,
1213-
conditional_message: Some("for the given month and year"),
1214-
});
1184+
return Err(error::ComponentRange::conditional("day"));
12151185
}
12161186
}
12171187

@@ -1244,13 +1214,7 @@ impl Date {
12441214
}
12451215
_ => {
12461216
hint::cold_path();
1247-
return Err(error::ComponentRange {
1248-
name: "day",
1249-
minimum: 1,
1250-
maximum: days_in_month_leap(self.month() as u8, is_leap_year) as i64,
1251-
value: day as i64,
1252-
conditional_message: Some("for the given month and year"),
1253-
});
1217+
return Err(error::ComponentRange::conditional("day"));
12541218
}
12551219
}
12561220

@@ -1281,13 +1245,7 @@ impl Date {
12811245
366 if is_leap_year => hint::cold_path(),
12821246
_ => {
12831247
hint::cold_path();
1284-
return Err(error::ComponentRange {
1285-
name: "ordinal",
1286-
minimum: 1,
1287-
maximum: if is_leap_year { 366 } else { 365 },
1288-
value: ordinal as i64,
1289-
conditional_message: Some("for the given year"),
1290-
});
1248+
return Err(error::ComponentRange::conditional("ordinal"));
12911249
}
12921250
}
12931251

‎time/src/error/component_range.rs‎

Lines changed: 30 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,80 +1,58 @@
11
//! Component range error
22
3-
use core::{fmt, hash};
3+
use core::fmt;
44

55
use crate::error;
66

77
/// An error type indicating that a component provided to a method was out of range, causing a
88
/// failure.
99
// i64 is the narrowest type fitting all use cases. This eliminates the need for a type parameter.
10-
#[derive(Debug, Clone, Copy, Eq)]
10+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
1111
pub struct ComponentRange {
1212
/// Name of the component.
1313
pub(crate) name: &'static str,
14-
/// Minimum allowed value, inclusive.
15-
pub(crate) minimum: i64,
16-
/// Maximum allowed value, inclusive.
17-
pub(crate) maximum: i64,
18-
/// Value that was provided.
19-
pub(crate) value: i64,
20-
/// The minimum and/or maximum value is conditional on the value of other
21-
/// parameters.
22-
pub(crate) conditional_message: Option<&'static str>,
14+
/// Whether an input with the same value could have succeeded if the values of other components
15+
/// were different.
16+
pub(crate) is_conditional: bool,
2317
}
2418

2519
impl ComponentRange {
26-
/// Obtain the name of the component whose value was out of range.
20+
/// Create a new `ComponentRange` error that is not conditional.
2721
#[inline]
28-
pub const fn name(self) -> &'static str {
29-
self.name
22+
pub(crate) const fn unconditional(name: &'static str) -> Self {
23+
Self {
24+
name,
25+
is_conditional: false,
26+
}
3027
}
3128

32-
/// Whether the value's permitted range is conditional, i.e. whether an input with this
33-
/// value could have succeeded if the values of other components were different.
29+
/// Create a new `ComponentRange` error that is conditional.
3430
#[inline]
35-
pub const fn is_conditional(self) -> bool {
36-
self.conditional_message.is_some()
31+
pub(crate) const fn conditional(name: &'static str) -> Self {
32+
Self {
33+
name,
34+
is_conditional: true,
35+
}
3736
}
38-
}
3937

40-
impl PartialEq for ComponentRange {
38+
/// Obtain the name of the component whose value was out of range.
4139
#[inline]
42-
fn eq(&self, other: &Self) -> bool {
43-
self.name == other.name
44-
&& self.minimum == other.minimum
45-
&& self.maximum == other.maximum
46-
&& self.value == other.value
47-
// Skip the contents of the message when comparing for equality.
48-
&& self.conditional_message.is_some() == other.conditional_message.is_some()
40+
pub const fn name(self) -> &'static str {
41+
self.name
4942
}
50-
}
5143

52-
impl hash::Hash for ComponentRange {
44+
/// Whether the value's permitted range is conditional, i.e. whether an input with this
45+
/// value could have succeeded if the values of other components were different.
5346
#[inline]
54-
fn hash<H: hash::Hasher>(&self, state: &mut H) {
55-
self.name.hash(state);
56-
self.minimum.hash(state);
57-
self.maximum.hash(state);
58-
self.value.hash(state);
59-
// Skip the contents of the message when comparing for equality.
60-
self.conditional_message.is_some().hash(state);
47+
pub const fn is_conditional(self) -> bool {
48+
self.is_conditional
6149
}
6250
}
6351

6452
impl fmt::Display for ComponentRange {
6553
#[inline]
6654
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
67-
write!(
68-
f,
69-
"{} must be in the range {}..={}",
70-
self.name, self.minimum, self.maximum
71-
)?;
72-
73-
if let Some(message) = self.conditional_message {
74-
write!(f, " {message}")?;
75-
}
76-
77-
Ok(())
55+
write!(f, "{} was not in range", self.name)
7856
}
7957
}
8058

@@ -102,11 +80,7 @@ impl TryFrom<crate::Error> for ComponentRange {
10280
impl serde_core::de::Expected for ComponentRange {
10381
#[inline]
10482
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
105-
write!(
106-
f,
107-
"a value in the range {}..={}",
108-
self.minimum, self.maximum
109-
)
83+
f.write_str("an in-range value")
11084
}
11185
}
11286

@@ -115,7 +89,10 @@ impl ComponentRange {
11589
/// Convert the error to a deserialization error.
11690
#[inline]
11791
pub(crate) fn into_de_error<E: serde_core::de::Error>(self) -> E {
118-
E::invalid_value(serde_core::de::Unexpected::Signed(self.value), &self)
92+
serde_core::de::Error::custom(format_args!(
93+
"invalid {}, expected an in-range value",
94+
self.name
95+
))
11996
}
12097
}
12198

‎time/src/formatting/mod.rs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -369,14 +369,7 @@ fn fmt_year(
369369
} else {
370370
match repr {
371371
modifier::YearRepr::Full | modifier::YearRepr::Century if full_year.abs() >= 10_000 => {
372-
return Err(error::ComponentRange {
373-
name: "year",
374-
minimum: -9999,
375-
maximum: 9999,
376-
value: full_year.extend(),
377-
conditional_message: Some("when `range:standard` is used"),
378-
}
379-
.into());
372+
return Err(error::ComponentRange::conditional("year").into());
380373
}
381374
_ => {}
382375
}

‎time/src/internal_macros.rs‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -172,14 +172,17 @@ macro_rules! ensure_ranged {
172172
Some(val) => val,
173173
None => {
174174
$crate::hint::cold_path();
175-
#[allow(trivial_numeric_casts)]
176-
return Err(crate::error::ComponentRange {
177-
name: stringify!($value),
178-
minimum: <$type>::MIN.get() as i64,
179-
maximum: <$type>::MAX.get() as i64,
180-
value: $value as i64,
181-
conditional_message: None,
182-
});
175+
return Err(crate::error::ComponentRange::unconditional(stringify!($value)));
176+
}
177+
}
178+
};
179+
180+
($type:ty : $value:ident ($name:literal)) => {
181+
match <$type>::new($value) {
182+
Some(val) => val,
183+
None => {
184+
$crate::hint::cold_path();
185+
return Err(crate::error::ComponentRange::unconditional($name));
183186
}
184187
}
185188
};
@@ -190,25 +193,12 @@ macro_rules! ensure_ranged {
190193
Some(val) => val,
191194
None => {
192195
$crate::hint::cold_path();
193-
#[allow(trivial_numeric_casts)]
194-
return Err(crate::error::ComponentRange {
195-
name: stringify!($value),
196-
minimum: <$type>::MIN.get() as i64 / $factor as i64,
197-
maximum: <$type>::MAX.get() as i64 / $factor as i64,
198-
value: $value as i64,
199-
conditional_message: None,
200-
});
196+
return Err(crate::error::ComponentRange::unconditional(stringify!($value)));
201197
}
202198
},
203199
None => {
204200
$crate::hint::cold_path();
205-
return Err(crate::error::ComponentRange {
206-
name: stringify!($value),
207-
minimum: <$type>::MIN.get() as i64 / $factor as i64,
208-
maximum: <$type>::MAX.get() as i64 / $factor as i64,
209-
value: $value as i64,
210-
conditional_message: None,
211-
});
201+
return Err(crate::error::ComponentRange::unconditional(stringify!($value)));
212202
}
213203
}
214204
};

‎time/src/month.rs‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,7 @@ impl Month {
5656
10 => Ok(October),
5757
11 => Ok(November),
5858
12 => Ok(December),
59-
n => Err(error::ComponentRange {
60-
name: "month",
61-
minimum: 1,
62-
maximum: 12,
63-
value: n as i64,
64-
conditional_message: None,
65-
}),
59+
_ => Err(error::ComponentRange::unconditional("month")),
6660
}
6761
}
6862

@@ -273,13 +267,7 @@ impl TryFrom<u8> for Month {
273267
fn try_from(value: u8) -> Result<Self, Self::Error> {
274268
match NonZero::new(value) {
275269
Some(value) => Self::from_number(value),
276-
None => Err(error::ComponentRange {
277-
name: "month",
278-
minimum: 1,
279-
maximum: 12,
280-
value: 0,
281-
conditional_message: None,
282-
}),
270+
None => Err(error::ComponentRange::unconditional("month")),
283271
}
284272
}
285273
}

‎time/src/parsing/parsable.rs‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -494,13 +494,7 @@ impl sealed::Sealed for Rfc2822 {
494494

495495
if leap_second_input && !dt.is_valid_leap_second_stand_in() {
496496
return Err(error::Parse::TryFromParsed(TryFromParsed::ComponentRange(
497-
error::ComponentRange {
498-
name: "second",
499-
minimum: 0,
500-
maximum: 59,
501-
value: 60,
502-
conditional_message: Some("because leap seconds are not supported"),
503-
},
497+
error::ComponentRange::conditional("second"),
504498
)));
505499
}
506500

@@ -696,15 +690,6 @@ impl sealed::Sealed for Rfc3339 {
696690
),
697691
}
698692
.map(|offset| ParsedItem(input, offset))
699-
.map_err(|mut err| {
700-
// Provide the user a more accurate error.
701-
if err.name == "hours" {
702-
err.name = "offset hour";
703-
} else if err.name == "minutes" {
704-
err.name = "offset minute";
705-
}
706-
err
707-
})
708693
.map_err(TryFromParsed::ComponentRange)?
709694
}
710695
};
@@ -735,13 +720,7 @@ impl sealed::Sealed for Rfc3339 {
735720

736721
if leap_second_input && !dt.is_valid_leap_second_stand_in() {
737722
return Err(error::Parse::TryFromParsed(TryFromParsed::ComponentRange(
738-
error::ComponentRange {
739-
name: "second",
740-
minimum: 0,
741-
maximum: 59,
742-
value: 60,
743-
conditional_message: Some("because leap seconds are not supported"),
744-
},
723+
error::ComponentRange::conditional("second"),
745724
)));
746725
}
747726

0 commit comments

Comments
 (0)
⚡