Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

large_enum_variant triggered in error-chain macro has unhelpful help message #2121

Open
killercup opened this issue Oct 9, 2017 · 8 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types

Comments

@killercup
Copy link
Member

You know how I like to suggest E-hard new lints? Sorry, this time I only have an E-hard L-enhancement to report.

warning: large size difference between variants
  --> src/main.rs:66:1
   |
66 | / error_chain! {
67 | |     foreign_links {
68 | |         Io(::std::io::Error);
69 | |         Handlebars(::handlebars::TemplateRenderError);
70 | |         Yaml(::yaml::Error);
71 | |     }
72 | | }
   | |_^
   |
   = note: #[warn(large_enum_variant)] on by default
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.165/index.html#large_enum_variant
help: consider boxing the large fields to reduce the total size of the enum
   |
113| err : Box<$ foreign_link_error_path> ) {
   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: this error originates in a macro outside of the current crate
   = issue author's remark: this help message is not helpful

Is there a way to (easily) print the enum variants with their sizes? This might be helpful even in rather obvious cases.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 10, 2017

is there a point in reporting anything inside that macro? We can just turn it off in macros.

@killercup
Copy link
Member Author

Well, the concern is valid: It's a large enum. Question is, if the programmer can do anything about it (in this case: probably). It's just the quoted code that's not really useful here.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 10, 2017

can you tell error_chain to box the args?

Something like

error_chain! {
    foreign_links {
        Io(Box<::std::io::Error>);
    }
}

?

@oli-obk oli-obk added good-first-issue These issues are a good way to get started with Clippy E-medium Call for participation: Medium difficulty level problem and requires some initial experience. C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages T-middle Type: Probably requires verifiying types labels Oct 10, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Oct 10, 2017

Is there a way to (easily) print the enum variants with their sizes? This might be helpful even in rather obvious cases.

That is trivial.

Figuring out how to produce nice error spans is a little involved, but sound like "just some detective work"

@killercup
Copy link
Member Author

can you tell error_chain to box the args?

Doesn't look like it: https://play.rust-lang.org/?gist=0007d7af1676d861d6cb534f4fa735c1&version=stable


Sorry, this time I only have an E-hard L-enhancement to report.

oli-obk added E-easy E-medium L-enhancement T-middle

😞

@oli-obk
Copy link
Contributor

oli-obk commented Oct 10, 2017

error-chain already seems to box its contents: https://play.rust-lang.org/?gist=232208cd5ee6f1fcf95c4eb52562dfde&version=stable

@killercup
Copy link
Member Author

Hahaha, sorry, hadn't had enough coffee yet. That's basically the quoted macro line from above! 😆

113| err : Box<$ foreign_link_error_path> ) {

@Arnavion
Copy link
Contributor

@killercup

can you tell error_chain to box the args?

Doesn't look like it: https://play.rust-lang.org/?gist=0007d7af1676d861d6cb534f4fa735c1&version=stable

error-chain is totally fine with letting you have a foreign link of Box<io::Error>, since Box<io::Error> does impl std::error::Error that foreign links require.

The error in your link is because you told error-chain that the foreign link is over Box<io::Error>, so the EK impls From<Box<io::Error>>. But your code is trying to use ? with an unboxed io::Error for which there is no From impl.


@oli-obk

error-chain already seems to box its contents: https://play.rust-lang.org/?gist=232208cd5ee6f1fcf95c4eb52562dfde&version=stable

No, error-chain's codegen does not box foreign links. The size of foo::Error and bar::Error in your playground is the same because of the effect of the implicit Msg(String) member that occupies 24 bytes, leading to 32 bytes total to include the discriminant. See:

mod foo {
    pub enum ErrorKind {
        Msg(String),
        Io(Box<::std::io::Error>),
        #[doc(hidden)]
        __Nonexhaustive { },
    }
}

mod bar {
    pub enum ErrorKind {
        Msg(String),
        Io(::std::io::Error),
        #[doc(hidden)]
        __Nonexhaustive { },
    }
}


fn main() {
    println!("{}", std::mem::size_of::<foo::ErrorKind>()); // 32
    println!("{}", std::mem::size_of::<bar::ErrorKind>()); // 32
    println!("{}", std::mem::size_of::<String>()); // 24
    println!("{}", std::mem::size_of::<::std::io::Error>()); // 16
    println!("{}", std::mem::size_of::<Box<::std::io::Error>>()); // 8
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

3 participants