From 6c0095799df8f571ebad3fd5681cf339dde64b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 13 May 2026 16:56:14 +0900 Subject: [PATCH 1/3] fix(error): box diagnostic metadata to shrink Error size Move context, location, and source into a heap-allocated ErrorMeta struct so that Error 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. --- clippy.toml | 7 -- crates/ironrdp-error/src/lib.rs | 115 +++++++++++++++++++++----------- 2 files changed, 76 insertions(+), 46 deletions(-) diff --git a/clippy.toml b/clippy.toml index 1e02e51c0..17e7e90fd 100644 --- a/clippy.toml +++ b/clippy.toml @@ -4,10 +4,3 @@ accept-comment-above-statement = true accept-comment-above-attributes = true allow-panic-in-tests = true allow-unwrap-in-tests = true -# Raised from the default of 128 to accommodate the `&'static core::panic::Location<'static>` -# field that `ironrdp_error::Error` captures via `#[track_caller]` for diagnostic -# reporting. The location field adds one usize (8 bytes) to every error type, which pushes -# `Error` (whose largest variant nests another `Error`) just over -# the 128-byte threshold. The trade-off favours diagnostic quality over the small -# Result-Ok-path cost. -large-error-threshold = 152 diff --git a/crates/ironrdp-error/src/lib.rs b/crates/ironrdp-error/src/lib.rs index f6132fad4..01f19d10c 100644 --- a/crates/ironrdp-error/src/lib.rs +++ b/crates/ironrdp-error/src/lib.rs @@ -9,28 +9,41 @@ extern crate alloc; use alloc::boxed::Box; use core::fmt; -#[cfg(feature = "std")] -pub trait Source: core::error::Error + Sync + Send + 'static {} - -#[cfg(feature = "std")] -impl Source for T where T: core::error::Error + Sync + Send + 'static {} - -#[cfg(not(feature = "std"))] -pub trait Source: fmt::Display + fmt::Debug + Send + Sync + 'static {} +pub trait Source: core::error::Error + Send + Sync + 'static {} -#[cfg(not(feature = "std"))] -impl Source for T where T: fmt::Display + fmt::Debug + Send + Sync + 'static {} +impl Source for T where T: core::error::Error + Send + Sync + 'static {} -pub struct Error { +/// Diagnostic metadata stored behind a [`Box`] so that `Error` stays small. +/// +/// All fields here are purely for display and error-chain traversal; none are +/// needed for matching on the error kind. The allocation only occurs when an +/// error is *constructed* — a cold path — so the per-error heap cost is +/// acceptable. +#[cfg(feature = "alloc")] +struct ErrorMeta { context: &'static str, location: &'static core::panic::Location<'static>, - kind: Kind, - #[cfg(feature = "std")] - source: Option>, - #[cfg(all(not(feature = "std"), feature = "alloc"))] source: Option>, } +/// A typed error wrapper carrying a `Kind` discriminant plus diagnostic metadata. +/// +/// # `no_alloc` platforms +/// +/// When compiled without the `alloc` feature, `Error` retains only +/// `kind` and `context`; the call-site location and error source chain are +/// unavailable. This is intentional: `no_alloc` targets are supported on a +/// best-effort basis and are not a primary target of this crate. +pub struct Error { + kind: Kind, + /// Diagnostic metadata. Present only when `alloc` is available. + #[cfg(feature = "alloc")] + meta: Box, + /// Minimal context kept for `no_alloc` targets (no location, no source). + #[cfg(not(feature = "alloc"))] + context: &'static str, +} + // Manual `Debug` impl that excludes the `location` field. The location is // captured via `core::panic::Location::caller()` and rendered in `Display`, // but its `file()` returns platform-native paths (`/` on Unix, `\` on @@ -39,9 +52,12 @@ pub struct Error { impl fmt::Debug for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut dbg = f.debug_struct("Error"); + #[cfg(feature = "alloc")] + dbg.field("context", &self.meta.context) + .field("kind", &self.kind) + .field("source", &self.meta.source); + #[cfg(not(feature = "alloc"))] dbg.field("context", &self.context).field("kind", &self.kind); - #[cfg(any(feature = "std", feature = "alloc"))] - dbg.field("source", &self.source); dbg.finish() } } @@ -52,11 +68,15 @@ impl Error { #[track_caller] pub fn new(context: &'static str, kind: Kind) -> Self { Self { - context, - location: core::panic::Location::caller(), kind, #[cfg(feature = "alloc")] - source: None, + meta: Box::new(ErrorMeta { + context, + location: core::panic::Location::caller(), + source: None, + }), + #[cfg(not(feature = "alloc"))] + context, } } @@ -69,11 +89,11 @@ impl Error { #[cfg(feature = "alloc")] { let mut this = self; - this.source = Some(Box::new(source)); + this.meta.source = Some(Box::new(source)); this } - // No source when no std and no alloc crates + // No source when no alloc #[cfg(not(feature = "alloc"))] { let _ = source; @@ -86,11 +106,11 @@ impl Error { Kind: Into, { Error { - context: self.context, - location: self.location, kind: self.kind.into(), - #[cfg(any(feature = "std", feature = "alloc"))] - source: self.source, + #[cfg(feature = "alloc")] + meta: self.meta, + #[cfg(not(feature = "alloc"))] + context: self.context, } } @@ -103,12 +123,22 @@ impl Error { /// Captured automatically by [`Error::new`] via [`core::panic::Location::caller`] /// and `#[track_caller]`. Useful for diagnostic logging and error reporting /// when the variant alone does not narrow down the call site enough. + /// + /// Not available on `no_alloc` platforms (see crate-level documentation). + #[cfg(feature = "alloc")] pub fn location(&self) -> &'static core::panic::Location<'static> { - self.location + self.meta.location } pub fn set_context(&mut self, context: &'static str) { - self.context = context; + #[cfg(feature = "alloc")] + { + self.meta.context = context; + } + #[cfg(not(feature = "alloc"))] + { + self.context = context; + } } pub fn report(&self) -> ErrorReport<'_, Kind> { @@ -121,14 +151,21 @@ where Kind: fmt::Display, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "[{} @ {}:{}] {}", - self.context, - self.location.file(), - self.location.line(), - self.kind - ) + #[cfg(feature = "alloc")] + { + write!( + f, + "[{} @ {}:{}] {}", + self.meta.context, + self.meta.location.file(), + self.meta.location.line(), + self.kind + ) + } + #[cfg(not(feature = "alloc"))] + { + write!(f, "[{}] {}", self.context, self.kind) + } } } @@ -141,8 +178,8 @@ where if let Some(source) = self.kind.source() { Some(source) } else { - // NOTE: we can’t use Option::as_ref here because of type inference - if let Some(e) = &self.source { + // NOTE: we can't use Option::as_ref here because of type inference + if let Some(e) = &self.meta.source { Some(e.as_ref()) } else { None @@ -193,7 +230,7 @@ where write!(f, "{}", self.0)?; #[cfg(feature = "alloc")] - if let Some(source) = &self.0.source { + if let Some(source) = &self.0.meta.source { write!(f, ", caused by: {source}")?; } From 57e4df42d287d08ff59d206ba466d7b8a2435552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 14 May 2026 18:31:21 +0900 Subject: [PATCH 2/3] . --- crates/ironrdp-error/src/lib.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/crates/ironrdp-error/src/lib.rs b/crates/ironrdp-error/src/lib.rs index 01f19d10c..10b49e7bb 100644 --- a/crates/ironrdp-error/src/lib.rs +++ b/crates/ironrdp-error/src/lib.rs @@ -30,18 +30,21 @@ struct ErrorMeta { /// /// # `no_alloc` platforms /// -/// When compiled without the `alloc` feature, `Error` retains only -/// `kind` and `context`; the call-site location and error source chain are -/// unavailable. This is intentional: `no_alloc` targets are supported on a -/// best-effort basis and are not a primary target of this crate. +/// When compiled without the `alloc` feature, `Error` retains `kind`, +/// `context`, and `location` inline. The error source chain is unavailable. +/// `no_alloc` targets are supported on a best-effort basis and are not a +/// primary target of this crate. Do not add more inline fields here: the +/// struct should stay lean for stack-constrained environments. pub struct Error { kind: Kind, /// Diagnostic metadata. Present only when `alloc` is available. #[cfg(feature = "alloc")] meta: Box, - /// Minimal context kept for `no_alloc` targets (no location, no source). + /// Minimal context kept for `no_alloc` targets (no source chain). #[cfg(not(feature = "alloc"))] context: &'static str, + #[cfg(not(feature = "alloc"))] + location: &'static core::panic::Location<'static>, } // Manual `Debug` impl that excludes the `location` field. The location is @@ -77,6 +80,8 @@ impl Error { }), #[cfg(not(feature = "alloc"))] context, + #[cfg(not(feature = "alloc"))] + location: core::panic::Location::caller(), } } @@ -111,6 +116,8 @@ impl Error { meta: self.meta, #[cfg(not(feature = "alloc"))] context: self.context, + #[cfg(not(feature = "alloc"))] + location: self.location, } } @@ -123,11 +130,15 @@ impl Error { /// Captured automatically by [`Error::new`] via [`core::panic::Location::caller`] /// and `#[track_caller]`. Useful for diagnostic logging and error reporting /// when the variant alone does not narrow down the call site enough. - /// - /// Not available on `no_alloc` platforms (see crate-level documentation). - #[cfg(feature = "alloc")] pub fn location(&self) -> &'static core::panic::Location<'static> { - self.meta.location + #[cfg(feature = "alloc")] + { + self.meta.location + } + #[cfg(not(feature = "alloc"))] + { + self.location + } } pub fn set_context(&mut self, context: &'static str) { @@ -164,7 +175,7 @@ where } #[cfg(not(feature = "alloc"))] { - write!(f, "[{}] {}", self.context, self.kind) + write!(f, "[{} @ {}:{}] {}", self.context, self.location.file(), self.location.line(), self.kind) } } } From 1827140d729bd72ec91a4be644b70341294f5390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 14 May 2026 18:32:20 +0900 Subject: [PATCH 3/3] . --- crates/ironrdp-error/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/ironrdp-error/src/lib.rs b/crates/ironrdp-error/src/lib.rs index 10b49e7bb..09a44f651 100644 --- a/crates/ironrdp-error/src/lib.rs +++ b/crates/ironrdp-error/src/lib.rs @@ -175,7 +175,14 @@ where } #[cfg(not(feature = "alloc"))] { - write!(f, "[{} @ {}:{}] {}", self.context, self.location.file(), self.location.line(), self.kind) + write!( + f, + "[{} @ {}:{}] {}", + self.context, + self.location.file(), + self.location.line(), + self.kind + ) } } }