Skip to content

Commit 6c00957

Browse files
committed
fix(error): box diagnostic metadata to shrink Error<Kind> size
Move context, location, and source into a heap-allocated ErrorMeta struct so that Error<Kind> holds only kind on the stack. This eliminates the size cascade that caused ConnectorError to exceed clippy's 128-byte threshold (it was raised to 152 to accommodate the Location field). Error construction is already #[cold], so the single Box allocation per error is acceptable.
1 parent 14c5502 commit 6c00957

2 files changed

Lines changed: 76 additions & 46 deletions

File tree

clippy.toml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,3 @@ accept-comment-above-statement = true
44
accept-comment-above-attributes = true
55
allow-panic-in-tests = true
66
allow-unwrap-in-tests = true
7-
# Raised from the default of 128 to accommodate the `&'static core::panic::Location<'static>`
8-
# field that `ironrdp_error::Error<Kind>` captures via `#[track_caller]` for diagnostic
9-
# reporting. The location field adds one usize (8 bytes) to every error type, which pushes
10-
# `Error<ConnectorErrorKind>` (whose largest variant nests another `Error<Kind>`) just over
11-
# the 128-byte threshold. The trade-off favours diagnostic quality over the small
12-
# Result-Ok-path cost.
13-
large-error-threshold = 152

crates/ironrdp-error/src/lib.rs

Lines changed: 76 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,41 @@ extern crate alloc;
99
use alloc::boxed::Box;
1010
use core::fmt;
1111

12-
#[cfg(feature = "std")]
13-
pub trait Source: core::error::Error + Sync + Send + 'static {}
14-
15-
#[cfg(feature = "std")]
16-
impl<T> Source for T where T: core::error::Error + Sync + Send + 'static {}
17-
18-
#[cfg(not(feature = "std"))]
19-
pub trait Source: fmt::Display + fmt::Debug + Send + Sync + 'static {}
12+
pub trait Source: core::error::Error + Send + Sync + 'static {}
2013

21-
#[cfg(not(feature = "std"))]
22-
impl<T> Source for T where T: fmt::Display + fmt::Debug + Send + Sync + 'static {}
14+
impl<T> Source for T where T: core::error::Error + Send + Sync + 'static {}
2315

24-
pub struct Error<Kind> {
16+
/// Diagnostic metadata stored behind a [`Box`] so that `Error<Kind>` stays small.
17+
///
18+
/// All fields here are purely for display and error-chain traversal; none are
19+
/// needed for matching on the error kind. The allocation only occurs when an
20+
/// error is *constructed* — a cold path — so the per-error heap cost is
21+
/// acceptable.
22+
#[cfg(feature = "alloc")]
23+
struct ErrorMeta {
2524
context: &'static str,
2625
location: &'static core::panic::Location<'static>,
27-
kind: Kind,
28-
#[cfg(feature = "std")]
29-
source: Option<Box<dyn core::error::Error + Sync + Send>>,
30-
#[cfg(all(not(feature = "std"), feature = "alloc"))]
3126
source: Option<Box<dyn Source>>,
3227
}
3328

29+
/// A typed error wrapper carrying a `Kind` discriminant plus diagnostic metadata.
30+
///
31+
/// # `no_alloc` platforms
32+
///
33+
/// When compiled without the `alloc` feature, `Error<Kind>` retains only
34+
/// `kind` and `context`; the call-site location and error source chain are
35+
/// unavailable. This is intentional: `no_alloc` targets are supported on a
36+
/// best-effort basis and are not a primary target of this crate.
37+
pub struct Error<Kind> {
38+
kind: Kind,
39+
/// Diagnostic metadata. Present only when `alloc` is available.
40+
#[cfg(feature = "alloc")]
41+
meta: Box<ErrorMeta>,
42+
/// Minimal context kept for `no_alloc` targets (no location, no source).
43+
#[cfg(not(feature = "alloc"))]
44+
context: &'static str,
45+
}
46+
3447
// Manual `Debug` impl that excludes the `location` field. The location is
3548
// captured via `core::panic::Location::caller()` and rendered in `Display`,
3649
// but its `file()` returns platform-native paths (`/` on Unix, `\` on
@@ -39,9 +52,12 @@ pub struct Error<Kind> {
3952
impl<Kind: fmt::Debug> fmt::Debug for Error<Kind> {
4053
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
4154
let mut dbg = f.debug_struct("Error");
55+
#[cfg(feature = "alloc")]
56+
dbg.field("context", &self.meta.context)
57+
.field("kind", &self.kind)
58+
.field("source", &self.meta.source);
59+
#[cfg(not(feature = "alloc"))]
4260
dbg.field("context", &self.context).field("kind", &self.kind);
43-
#[cfg(any(feature = "std", feature = "alloc"))]
44-
dbg.field("source", &self.source);
4561
dbg.finish()
4662
}
4763
}
@@ -52,11 +68,15 @@ impl<Kind> Error<Kind> {
5268
#[track_caller]
5369
pub fn new(context: &'static str, kind: Kind) -> Self {
5470
Self {
55-
context,
56-
location: core::panic::Location::caller(),
5771
kind,
5872
#[cfg(feature = "alloc")]
59-
source: None,
73+
meta: Box::new(ErrorMeta {
74+
context,
75+
location: core::panic::Location::caller(),
76+
source: None,
77+
}),
78+
#[cfg(not(feature = "alloc"))]
79+
context,
6080
}
6181
}
6282

@@ -69,11 +89,11 @@ impl<Kind> Error<Kind> {
6989
#[cfg(feature = "alloc")]
7090
{
7191
let mut this = self;
72-
this.source = Some(Box::new(source));
92+
this.meta.source = Some(Box::new(source));
7393
this
7494
}
7595

76-
// No source when no std and no alloc crates
96+
// No source when no alloc
7797
#[cfg(not(feature = "alloc"))]
7898
{
7999
let _ = source;
@@ -86,11 +106,11 @@ impl<Kind> Error<Kind> {
86106
Kind: Into<OtherKind>,
87107
{
88108
Error {
89-
context: self.context,
90-
location: self.location,
91109
kind: self.kind.into(),
92-
#[cfg(any(feature = "std", feature = "alloc"))]
93-
source: self.source,
110+
#[cfg(feature = "alloc")]
111+
meta: self.meta,
112+
#[cfg(not(feature = "alloc"))]
113+
context: self.context,
94114
}
95115
}
96116

@@ -103,12 +123,22 @@ impl<Kind> Error<Kind> {
103123
/// Captured automatically by [`Error::new`] via [`core::panic::Location::caller`]
104124
/// and `#[track_caller]`. Useful for diagnostic logging and error reporting
105125
/// when the variant alone does not narrow down the call site enough.
126+
///
127+
/// Not available on `no_alloc` platforms (see crate-level documentation).
128+
#[cfg(feature = "alloc")]
106129
pub fn location(&self) -> &'static core::panic::Location<'static> {
107-
self.location
130+
self.meta.location
108131
}
109132

110133
pub fn set_context(&mut self, context: &'static str) {
111-
self.context = context;
134+
#[cfg(feature = "alloc")]
135+
{
136+
self.meta.context = context;
137+
}
138+
#[cfg(not(feature = "alloc"))]
139+
{
140+
self.context = context;
141+
}
112142
}
113143

114144
pub fn report(&self) -> ErrorReport<'_, Kind> {
@@ -121,14 +151,21 @@ where
121151
Kind: fmt::Display,
122152
{
123153
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
124-
write!(
125-
f,
126-
"[{} @ {}:{}] {}",
127-
self.context,
128-
self.location.file(),
129-
self.location.line(),
130-
self.kind
131-
)
154+
#[cfg(feature = "alloc")]
155+
{
156+
write!(
157+
f,
158+
"[{} @ {}:{}] {}",
159+
self.meta.context,
160+
self.meta.location.file(),
161+
self.meta.location.line(),
162+
self.kind
163+
)
164+
}
165+
#[cfg(not(feature = "alloc"))]
166+
{
167+
write!(f, "[{}] {}", self.context, self.kind)
168+
}
132169
}
133170
}
134171

@@ -141,8 +178,8 @@ where
141178
if let Some(source) = self.kind.source() {
142179
Some(source)
143180
} else {
144-
// NOTE: we cant use Option::as_ref here because of type inference
145-
if let Some(e) = &self.source {
181+
// NOTE: we can't use Option::as_ref here because of type inference
182+
if let Some(e) = &self.meta.source {
146183
Some(e.as_ref())
147184
} else {
148185
None
@@ -193,7 +230,7 @@ where
193230
write!(f, "{}", self.0)?;
194231

195232
#[cfg(feature = "alloc")]
196-
if let Some(source) = &self.0.source {
233+
if let Some(source) = &self.0.meta.source {
197234
write!(f, ", caused by: {source}")?;
198235
}
199236

0 commit comments

Comments
 (0)