Skip to content

Commit a2aa60b

Browse files
committed
Decrease CallError size from 176 to 8 bytes
Using Box to reduce stack size, especially when used in Result<T, CallError> return types. Should be OK since CallError is on the error path. Notified by https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
1 parent 4d2251b commit a2aa60b

File tree

1 file changed

+42
-19
lines changed

1 file changed

+42
-19
lines changed

godot-core/src/meta/error/call_error.rs

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,14 @@ use std::fmt;
7171
/// assert_eq!(inner.class_name(), Some("MyClass"));
7272
/// assert_eq!(inner.method_name(), "my_method");
7373
/// }
74-
#[derive(Debug)]
7574
pub struct CallError {
75+
// Boxed since the original struct is >= 176 bytes, making Result<..., CallError> very large.
76+
b: Box<InnerCallError>,
77+
}
78+
79+
/// Inner struct. All functionality on outer `impl`.
80+
#[derive(Debug)]
81+
struct InnerCallError {
7682
class_name: String,
7783
function_name: String,
7884
call_expr: String,
@@ -92,16 +98,16 @@ impl CallError {
9298
/// This is the static and not the dynamic type. For example, if you invoke `call()` on a `Gd<Node>`, you are effectively invoking
9399
/// `Object::call()` (through `DerefMut`), and the class name will be `Object`.
94100
pub fn class_name(&self) -> Option<&str> {
95-
if self.class_name.is_empty() {
101+
if self.b.class_name.is_empty() {
96102
None
97103
} else {
98-
Some(&self.class_name)
104+
Some(&self.b.class_name)
99105
}
100106
}
101107

102108
/// Name of the function or method that failed.
103109
pub fn method_name(&self) -> &str {
104-
&self.function_name
110+
&self.b.function_name
105111
}
106112

107113
// ------------------------------------------------------------------------------------------------------------------------------------------
@@ -150,8 +156,7 @@ impl CallError {
150156
// If the call error encodes an error generated by us, decode it.
151157
let mut source_error = None;
152158
if err.error == sys::GODOT_RUST_CUSTOM_CALL_ERROR {
153-
source_error = crate::private::call_error_remove(&err) //.
154-
.map(|e| SourceError::Call(Box::new(e)));
159+
source_error = crate::private::call_error_remove(&err).map(SourceError::Call);
155160
}
156161

157162
Err(Self::failed_varcall_inner(
@@ -295,8 +300,8 @@ impl CallError {
295300
// source,
296301
// }
297302

298-
call_error.source = source;
299-
call_error.call_expr = call_expr;
303+
call_error.b.source = source;
304+
call_error.b.call_expr = call_expr;
300305
call_error
301306
}
302307

@@ -316,7 +321,7 @@ impl CallError {
316321
reason: impl Into<String>,
317322
source: Option<ConvertError>,
318323
) -> Self {
319-
Self {
324+
let inner = InnerCallError {
320325
class_name: call_ctx.class_name.to_string(),
321326
function_name: call_ctx.function_name.to_string(),
322327
call_expr: format!("{call_ctx}()"),
@@ -325,17 +330,22 @@ impl CallError {
325330
value: e.value().map_or_else(String::new, |v| format!("{:?}", v)),
326331
erased_error: e.into(),
327332
}),
328-
}
333+
};
334+
335+
Self { b: Box::new(inner) }
329336
}
330337

331338
/// Describes the error.
332339
///
333340
/// This is the same as the `Display`/`ToString` repr, but without the prefix mentioning that this is a function call error,
334341
/// and without any source error information.
335-
fn message(&self, with_source: bool) -> String {
336-
let Self {
337-
call_expr, reason, ..
338-
} = self;
342+
pub fn message(&self, with_source: bool) -> String {
343+
let InnerCallError {
344+
call_expr,
345+
reason,
346+
source,
347+
..
348+
} = &*self.b;
339349

340350
let reason_str = if reason.is_empty() {
341351
String::new()
@@ -351,7 +361,7 @@ impl CallError {
351361
// String::new()
352362
// };
353363

354-
let source_str = match &self.source {
364+
let source_str = match source {
355365
Some(SourceError::Convert {
356366
erased_error,
357367
value,
@@ -361,7 +371,10 @@ impl CallError {
361371
if value.is_empty() { "" } else { ": " },
362372
)
363373
}
364-
Some(SourceError::Call(e)) if with_source => format!("\n Source: {}", e.message(true)),
374+
Some(SourceError::Call(e)) if with_source => {
375+
let message = e.message(true);
376+
format!("\n Source: {message}")
377+
}
365378
_ => String::new(),
366379
};
367380

@@ -371,13 +384,21 @@ impl CallError {
371384

372385
impl fmt::Display for CallError {
373386
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
374-
write!(f, "godot-rust function call failed: {}", self.message(true))
387+
let message = self.message(true);
388+
write!(f, "godot-rust function call failed: {message}")
389+
}
390+
}
391+
392+
impl fmt::Debug for CallError {
393+
// Delegate to inner box.
394+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
395+
write!(f, "{:?}", self.b)
375396
}
376397
}
377398

378399
impl Error for CallError {
379400
fn source(&self) -> Option<&(dyn Error + 'static)> {
380-
match self.source.as_ref() {
401+
match self.b.source.as_ref() {
381402
Some(SourceError::Convert {
382403
erased_error: e, ..
383404
}) => deref_to::<ErasedConvertError>(e),
@@ -396,7 +417,9 @@ enum SourceError {
396417
erased_error: ErasedConvertError,
397418
value: String,
398419
},
399-
Call(Box<CallError>),
420+
421+
// If the top-level Box on CallError is ever removed, this would need to store Box<CallError> again.
422+
Call(CallError),
400423
}
401424

402425
/// Explicit dereferencing to a certain type. Avoids accidentally returning `&Box<T>` or so.

0 commit comments

Comments
 (0)