Skip to content

Commit 2e2699d

Browse files
authored
fix(error): box diagnostic metadata to shrink Error<Kind> size (Devolutions#1269)
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 df0bf9c commit 2e2699d

2 files changed

Lines changed: 92 additions & 44 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: 92 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -9,26 +9,42 @@ 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 {}
12+
pub trait Source: core::error::Error + Send + Sync + 'static {}
1713

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

21-
#[cfg(not(feature = "std"))]
22-
impl<T> Source for T where T: fmt::Display + fmt::Debug + Send + Sync + 'static {}
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 {
24+
context: &'static str,
25+
location: &'static core::panic::Location<'static>,
26+
source: Option<Box<dyn Source>>,
27+
}
2328

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 `kind`,
34+
/// `context`, and `location` inline. The error source chain is unavailable.
35+
/// `no_alloc` targets are supported on a best-effort basis and are not a
36+
/// primary target of this crate. Do not add more inline fields here: the
37+
/// struct should stay lean for stack-constrained environments.
2438
pub struct Error<Kind> {
39+
kind: Kind,
40+
/// Diagnostic metadata. Present only when `alloc` is available.
41+
#[cfg(feature = "alloc")]
42+
meta: Box<ErrorMeta>,
43+
/// Minimal context kept for `no_alloc` targets (no source chain).
44+
#[cfg(not(feature = "alloc"))]
2545
context: &'static str,
46+
#[cfg(not(feature = "alloc"))]
2647
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"))]
31-
source: Option<Box<dyn Source>>,
3248
}
3349

3450
// Manual `Debug` impl that excludes the `location` field. The location is
@@ -39,9 +55,12 @@ pub struct Error<Kind> {
3955
impl<Kind: fmt::Debug> fmt::Debug for Error<Kind> {
4056
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
4157
let mut dbg = f.debug_struct("Error");
58+
#[cfg(feature = "alloc")]
59+
dbg.field("context", &self.meta.context)
60+
.field("kind", &self.kind)
61+
.field("source", &self.meta.source);
62+
#[cfg(not(feature = "alloc"))]
4263
dbg.field("context", &self.context).field("kind", &self.kind);
43-
#[cfg(any(feature = "std", feature = "alloc"))]
44-
dbg.field("source", &self.source);
4564
dbg.finish()
4665
}
4766
}
@@ -52,11 +71,17 @@ impl<Kind> Error<Kind> {
5271
#[track_caller]
5372
pub fn new(context: &'static str, kind: Kind) -> Self {
5473
Self {
55-
context,
56-
location: core::panic::Location::caller(),
5774
kind,
5875
#[cfg(feature = "alloc")]
59-
source: None,
76+
meta: Box::new(ErrorMeta {
77+
context,
78+
location: core::panic::Location::caller(),
79+
source: None,
80+
}),
81+
#[cfg(not(feature = "alloc"))]
82+
context,
83+
#[cfg(not(feature = "alloc"))]
84+
location: core::panic::Location::caller(),
6085
}
6186
}
6287

@@ -69,11 +94,11 @@ impl<Kind> Error<Kind> {
6994
#[cfg(feature = "alloc")]
7095
{
7196
let mut this = self;
72-
this.source = Some(Box::new(source));
97+
this.meta.source = Some(Box::new(source));
7398
this
7499
}
75100

76-
// No source when no std and no alloc crates
101+
// No source when no alloc
77102
#[cfg(not(feature = "alloc"))]
78103
{
79104
let _ = source;
@@ -86,11 +111,13 @@ impl<Kind> Error<Kind> {
86111
Kind: Into<OtherKind>,
87112
{
88113
Error {
114+
kind: self.kind.into(),
115+
#[cfg(feature = "alloc")]
116+
meta: self.meta,
117+
#[cfg(not(feature = "alloc"))]
89118
context: self.context,
119+
#[cfg(not(feature = "alloc"))]
90120
location: self.location,
91-
kind: self.kind.into(),
92-
#[cfg(any(feature = "std", feature = "alloc"))]
93-
source: self.source,
94121
}
95122
}
96123

@@ -104,11 +131,25 @@ impl<Kind> Error<Kind> {
104131
/// and `#[track_caller]`. Useful for diagnostic logging and error reporting
105132
/// when the variant alone does not narrow down the call site enough.
106133
pub fn location(&self) -> &'static core::panic::Location<'static> {
107-
self.location
134+
#[cfg(feature = "alloc")]
135+
{
136+
self.meta.location
137+
}
138+
#[cfg(not(feature = "alloc"))]
139+
{
140+
self.location
141+
}
108142
}
109143

110144
pub fn set_context(&mut self, context: &'static str) {
111-
self.context = context;
145+
#[cfg(feature = "alloc")]
146+
{
147+
self.meta.context = context;
148+
}
149+
#[cfg(not(feature = "alloc"))]
150+
{
151+
self.context = context;
152+
}
112153
}
113154

114155
pub fn report(&self) -> ErrorReport<'_, Kind> {
@@ -121,14 +162,28 @@ where
121162
Kind: fmt::Display,
122163
{
123164
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-
)
165+
#[cfg(feature = "alloc")]
166+
{
167+
write!(
168+
f,
169+
"[{} @ {}:{}] {}",
170+
self.meta.context,
171+
self.meta.location.file(),
172+
self.meta.location.line(),
173+
self.kind
174+
)
175+
}
176+
#[cfg(not(feature = "alloc"))]
177+
{
178+
write!(
179+
f,
180+
"[{} @ {}:{}] {}",
181+
self.context,
182+
self.location.file(),
183+
self.location.line(),
184+
self.kind
185+
)
186+
}
132187
}
133188
}
134189

@@ -141,8 +196,8 @@ where
141196
if let Some(source) = self.kind.source() {
142197
Some(source)
143198
} else {
144-
// NOTE: we cant use Option::as_ref here because of type inference
145-
if let Some(e) = &self.source {
199+
// NOTE: we can't use Option::as_ref here because of type inference
200+
if let Some(e) = &self.meta.source {
146201
Some(e.as_ref())
147202
} else {
148203
None
@@ -193,7 +248,7 @@ where
193248
write!(f, "{}", self.0)?;
194249

195250
#[cfg(feature = "alloc")]
196-
if let Some(source) = &self.0.source {
251+
if let Some(source) = &self.0.meta.source {
197252
write!(f, ", caused by: {source}")?;
198253
}
199254

0 commit comments

Comments
 (0)