Skip to content

Would it be so bad if you could opt into #[from] generating From<..> for Box<..>? #418

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

Closed
wants to merge 1 commit into from

Conversation

daniel-levin
Copy link

Hi @dtolnay! Long time listener, first time caller. Please, step into my bikeshed.

I'm looking to change your mind about #57, #302 and #415. I think I understand your position, and I'd like to share mine. I thought that actually bothering to take a crack at it would give me an idea of how burdensome this feature would be to thiserror's users and maintainer.

Anyhow (hah!), I've put together a crude first pass of a new opt-in feature called autobox, that, well, automatically boxes thiserror-derived (enum only for now) errors, as shown in the new test code:

#[derive(Debug, Error)]
enum ErrorEnum {
    #[error("bad")]
    Any(#[from] anyhow::Error),

    #[error("worse")]
    Big(#[from] VeryLargeError),
}

/// External code may still return large errors...
fn do_something() -> Result<(), VeryLargeError> {
    Err(VeryLargeError::new())
}

/// But we should be able to box them automatically!
fn do_something_else() -> Result<(), Box<ErrorEnum>> {
    do_something()?;

    Ok(())
}

I am sure that you have opinions on the implementation details, but I'm here to argue for this feature being desirable in thiserror, even though you've said you don't want it.

Firstly, like the existing #[from] implementations, the new feature and derived code is entirely opt in. Indeed, the new code is masked out by a feature flag, so the proc macro does nothing new unless you explicitly ask it to. As such, the new impls won't ever conflict with existing into-box implementations, whether hand-written or derived by another macro.

Secondly, I'd argue that now that Clippy's result_large_err is stabilized, more and more people are going to bump into this lint, and therefore there is some "public interest" in the existence of an easy solution that isn't hand-writing the from-for-box impl. Other proc macro crates could certainly achieve the same thing, yes. But, no existing crate, not even derive_more, is as convenient or widely trusted as thiserror. I don't want to take a dependency on yet another crate for a fairly trivial impl.

Finally, I'd like to say that this new feature doesn't actually require non-trivial changes to thiserrors internals, so far as I can tell.

As an aside, I'd like to disarm the anti-Rust crowd who in the future will inevitably wheel out the "look at all those needless memcpys" strawman.

@dtolnay
Copy link
Owner

dtolnay commented Jun 8, 2025

the new impls won't ever conflict with existing into-box implementations, whether hand-written or derived by another macro.

From reading the PR, I do not understand how this is the case. If a crate has an into-box implementation, handwritten or derived by another macro, and some unrelated part of the dependency graph turns on this feature, that crate will break.

@daniel-levin
Copy link
Author

Hmmm, I'm not very familiar with the way feature flags work with transitive proc macro crates. Regardless it now strikes me as a bad idea to turn this on for every error in a crate with a feature flag. An argument to the from attribute would be better. I'll do some more experimenting in the morning.

@dtolnay
Copy link
Owner

dtolnay commented Jun 8, 2025

Let's keep this draft for future reference, please open a new PR if you come up with a different strategy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants