Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Shrink the size of all Error types #225

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Shrink the size of all Error types #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Unreleased

- [Remove `impl Deref<Kind> for Error`](https://github.com/rust-lang-nursery/error-chain/pull/192)
- [Shrink the size of `Error` to a pointer](https://github.com/rust-lang-nursery/error-chain/pull/225)

# 0.11.0

Expand Down
6 changes: 3 additions & 3 deletions examples/chain_err.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ fn load_config(rel_path: &str) -> Result<()> {

/// Launch the service.
fn launch(rel_path: &str) -> Result<()> {
load_config(rel_path).map_err(|e| match e {
e @ Error(ErrorKind::ConfigLoad(_), _) => {
load_config(rel_path).map_err(|e| match *e.kind() {
ErrorKind::ConfigLoad(_) => {
e.chain_err(|| LaunchStage::ConfigLoad)
}
e => e.chain_err(|| "Unknown failure"),
_ => e.chain_err(|| "Unknown failure"),
})
}

Expand Down
30 changes: 0 additions & 30 deletions examples/size.rs
View file
Open in desktop

This file was deleted.

43 changes: 18 additions & 25 deletions src/error_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,13 @@ macro_rules! impl_error_chain_processed {
/// - a backtrace, generated when the error is created.
/// - an error chain, used for the implementation of `Error::cause()`.
#[derive(Debug)]
pub struct $error_name(
// The members must be `pub` for `links`.
/// The kind of the error.
pub $error_kind_name,
/// Contains the error chain and the backtrace.
#[doc(hidden)]
pub $crate::State,
);
pub struct $error_name(pub Box<($error_kind_name, $crate::State)>);
Copy link
Contributor

@Yamakaky Yamakaky Sep 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we can't pattern match on the kind (box if nightly only), maybe we could remove pub and put everything into $crate as

struct ErrorInternals<K> {
  kind: K,
  backtrace: Backtrace,
  next: ...
}

I tried that at one time, but it was rejected because it prohibited the use of pattern matching. But if we can't use it anyway...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in the usage I've seen I think it's quite important to match on ErrorKind, but is matching on the entire error itself that important?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes : https://github.com/rust-lang-nursery/error-chain/blob/master/src/lib.rs#L348-L353
With the new method, you can't do that anymore. You have to match on Result, then on error.kind().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong link. Adding #226

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, yes.


impl $crate::ChainedError for $error_name {
type ErrorKind = $error_kind_name;

fn new(kind: $error_kind_name, state: $crate::State) -> $error_name {
$error_name(kind, state)
$error_name(Box::new((kind, state)))
}

fn from_kind(kind: Self::ErrorKind) -> Self {
Expand Down Expand Up @@ -120,10 +113,10 @@ macro_rules! impl_error_chain_processed {
impl $error_name {
/// Constructs an error from a kind, and generates a backtrace.
pub fn from_kind(kind: $error_kind_name) -> $error_name {
$error_name(
$error_name(Box::new((
kind,
$crate::State::default(),
)
)))
}

/// Constructs a chained error from another error and a kind, and generates a backtrace.
Expand All @@ -140,15 +133,15 @@ macro_rules! impl_error_chain_processed {
-> $error_name
where K: Into<$error_kind_name>
{
$error_name(
$error_name(Box::new((
kind.into(),
$crate::State::new::<$error_name>(error, ),
)
)))
}

/// Returns the kind of the error.
pub fn kind(&self) -> &$error_kind_name {
&self.0
&(self.0).0
}

/// Iterates over the error chain.
Expand All @@ -158,7 +151,7 @@ macro_rules! impl_error_chain_processed {

/// Returns the backtrace associated with this error.
pub fn backtrace(&self) -> Option<&$crate::Backtrace> {
self.1.backtrace()
(self.0).1.backtrace()
}

/// Extends the error chain with a new entry.
Expand All @@ -170,7 +163,7 @@ macro_rules! impl_error_chain_processed {
/// A short description of the error.
/// This method is identical to [`Error::description()`](https://doc.rust-lang.org/nightly/std/error/trait.Error.html#tymethod.description)
pub fn description(&self) -> &str {
self.0.description()
(self.0).0.description()
}
}

Expand All @@ -181,10 +174,10 @@ macro_rules! impl_error_chain_processed {

#[allow(unknown_lints, unused_doc_comment)]
fn cause(&self) -> Option<&::std::error::Error> {
match self.1.next_error {
match (self.0).1.next_error {
Some(ref c) => Some(&**c),
None => {
match self.0 {
match (self.0).0 {
$(
$(#[$meta_foreign_links])*
$error_kind_name::$foreign_link_variant(ref foreign_err) => {
Expand All @@ -200,18 +193,19 @@ macro_rules! impl_error_chain_processed {

impl ::std::fmt::Display for $error_name {
fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
::std::fmt::Display::fmt(&self.0, f)
::std::fmt::Display::fmt(&(self.0).0, f)
}
}

$(
$(#[$meta_links])*
impl From<$link_error_path> for $error_name {
fn from(e: $link_error_path) -> Self {
$error_name(
let e = *e.0;
$error_name(Box::new((
$error_kind_name::$link_variant(e.0),
e.1,
)
)))
}
}
) *
Expand Down Expand Up @@ -245,7 +239,6 @@ macro_rules! impl_error_chain_processed {
}
}


// The ErrorKind type
// --------------

Expand Down Expand Up @@ -303,7 +296,7 @@ macro_rules! impl_error_chain_processed {

impl From<$error_name> for $error_kind_name {
fn from(e: $error_name) -> Self {
e.0
(e.0).0
}
}

Expand Down Expand Up @@ -425,13 +418,13 @@ macro_rules! impl_extract_backtrace {
fn extract_backtrace(e: &(::std::error::Error + Send + 'static))
-> Option<$crate::InternalBacktrace> {
if let Some(e) = e.downcast_ref::<$error_name>() {
return Some(e.1.backtrace.clone());
return Some((e.0).1.backtrace.clone());
}
$(
$( #[$meta_links] )*
{
if let Some(e) = e.downcast_ref::<$link_error_path>() {
return Some(e.1.backtrace.clone());
return Some((e.0).1.backtrace.clone());
}
}
) *
Expand Down
10 changes: 5 additions & 5 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,9 @@
//! }
//! }
//!
//! match Error::from("error!") {
//! Error(ErrorKind::InvalidToolchainName(_), _) => { }
//! Error(ErrorKind::Msg(_), _) => { }
//! match *Error::from("error!").kind() {
//! ErrorKind::InvalidToolchainName(_) => { }
//! ErrorKind::Msg(_) => { }
//! _ => { }
//! }
//! # }
Expand Down Expand Up @@ -377,8 +377,8 @@
//!
//!
//! # fn main() {
//! match app::Error::from("error!") {
//! app::Error(app::ErrorKind::Utils(utils::ErrorKind::BadStuff), _) => { }
//! match *app::Error::from("error!").kind() {
//! app::ErrorKind::Utils(utils::ErrorKind::BadStuff) => { }
//! _ => { }
//! }
//! # }
Expand Down
14 changes: 9 additions & 5 deletions tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ fn error_chain_err() {
let base = Error::from(ErrorKind::Test);
let ext = base.chain_err(|| "Test passes");

if let Error(ErrorKind::Msg(_), _) = ext {
if let ErrorKind::Msg(_) = *ext.kind() {
// pass
} else {
panic!("The error should be wrapped. {:?}", ext);
Expand Down Expand Up @@ -484,8 +484,8 @@ fn error_patterns() {
}

// Tuples look nice when matching errors
match Error::from("Test") {
Error(ErrorKind::Msg(_), _) => {},
match *Error::from("Test").kind() {
ErrorKind::Msg(_) => {},
_ => {},
}
}
Expand All @@ -500,8 +500,12 @@ fn result_match() {

match ok() {
Ok(()) => {},
Err(Error(ErrorKind::Msg(_), _)) => {},
Err(..) => {},
Err(e) => {
match *e.kind() {
ErrorKind::Msg(_) => {}
_ => {}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very satisfied with this new behaviour, it's less straightforward. Not sure how to improve it though...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

box_patterns will be stabilized some day, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yamakaky do you have a preferred method of how we might land this? I'd personally like to see this as at least an option, it's what I'd want for libraries 99% of the time I believe.

}
}

Expand Down