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

Generic type parameter on error #79

Closed
FintanH opened this issue Apr 24, 2020 · 15 comments · Fixed by #148
Closed

Generic type parameter on error #79

FintanH opened this issue Apr 24, 2020 · 15 comments · Fixed by #148

Comments

@FintanH
Copy link

FintanH commented Apr 24, 2020

I ran into an error when having a generic parameter in my error type. So the setup is that I want to have a variant in my error enum such that it delegates to a different error, like below:

use thiserror::Error;

#[derive(Debug, Clone, Error)]
enum Error<E> {
    #[error("variant error")]
    Variant,
    #[error(transparent)]
    Delegate(#[from] E),
}

Unfortunately, I get an error about E not having a Debug instance:

error[E0277]: `E` doesn't implement `std::fmt::Debug`
 --> src/main.rs:4:1
  |
4 | enum Error<E> {
  | ^^^^^^^^^^ - help: consider restricting this bound: `E: std::fmt::Debug`
  | |
  | `E` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
  |
  = help: the trait `std::fmt::Debug` is not implemented for `E`
  = note: required because of the requirements on the impl of `std::fmt::Debug` for `Error<E>`

Could this be possible to do or is there something that is restricting this use case? 🤔

For now, I'm going to write out the implementations by hand, but it would be cool to delegate that work to thiserror.

Thanks for any help 🙏

@dtolnay
Copy link
Owner

dtolnay commented Apr 24, 2020

The #[from] attribute (which implies #[source]) requires that the source error has a std::error::Error impl and Debug impl. For your use case would this work?—

use std::error::Error as StdError;
use thiserror::Error;

#[derive(Error, Debug)]
enum Error<E: StdError + 'static> {
    #[error("variant error")]
    Variant,
    #[error(transparent)]
    Delegate(#[from] E),
}

@FintanH
Copy link
Author

FintanH commented Apr 24, 2020

Ya, I didn't want to do that because then I have to litter the same bound everywhere I introduce E 😄

I was hoping that the generated code localise that like here:

impl<E: error::Error> error::Error for Error<E>

@dtolnay
Copy link
Owner

dtolnay commented Apr 24, 2020

The concern is an E: std::error::Error bound is too strong and doesn't describe the behavior of the generated Error impl. As an example the following currently compiles:

use std::error::Error as StdError;
use std::fmt::{Debug, Display};
use std::ops::Deref;
use thiserror::Error;

trait BoxedError: Debug + Display + Deref<Target = dyn StdError> {}
impl<E: Debug + Display + Deref<Target = dyn StdError>> BoxedError for E {}

#[derive(Error, Debug, Clone)]
enum Error<E: BoxedError> {
    #[error(transparent)]
    Delegate(#[from] E),
}

and works with Error<Box<dyn StdError>>, even though Box<dyn StdError> does not have a std::error::Error impl. Having the derive macro insert an additional E: std::error::Error bound on the Error impl in this case would be incorrect.

@dtolnay
Copy link
Owner

dtolnay commented Apr 24, 2020

I may consider accepting an attribute to place an additional bound on just the generated std::error::Error impl.

#[derive(Error, Debug)]
#[error(bound = std::error::Error + 'static)]
enum Error<E> {
    #[error(transparent)]
    Delegate(#[from] E),
}

@dtolnay
Copy link
Owner

dtolnay commented Apr 24, 2020

I suppose it would be fine to recognize the very specific case of enum Error<E> where E has no trait bounds in the enum definition and the enum has no where clause and E is used as a #[from] or #[source], in which case assume an E: std::error::Error + 'static bound on the generated Error impl as a reasonable default.

@FintanH
Copy link
Author

FintanH commented Apr 24, 2020

and works with Error<Box>, even though Box does not have a std::error::Error impl.

That's intriguing! I haven't seen anything set up like that before, but ya that makes sense :) Would not want to be overly restrictive.

I'm not sure what the advantages or disadvantages of the two above are, but I'd be happy with either 👍 If there's anything I could do to help with that I'd be happy to give it a shot at some point.

@dtolnay
Copy link
Owner

dtolnay commented Apr 24, 2020

If you're able to send a PR, I would gladly accept one for #79 (comment).

@FintanH
Copy link
Author

FintanH commented Apr 24, 2020

I'll take a crack at it when I get a chance :) I'll ping questions here if I get lost

@jeromegn
Copy link

jeromegn commented Oct 5, 2020

The example described in #79 (comment) does not work with multiple #[from].

For example:

#[derive(Debug, thiserror::Error)]
pub enum MyError<E: std::error::Error + 'static> {
    #[error("io: {0}")]
    Io(#[from] std::io::Error),
    #[error(transparent)]
    Inner(#[from] E),
}

returns the following error:

conflicting implementations of trait `std::convert::From<std::io::Error>` for type `MyError<std::io::Error>`

is this because E could also be a std::io::Error?

@FintanH
Copy link
Author

FintanH commented Oct 5, 2020

You're right @jeromegn. I didn't account for the generic parameter being allowed to be the same as an existing variant... So this might be not very feasible in the end :/

@vikigenius
Copy link

Is there anything preventing #107 from getting merged. Would love to be able to do this without having to write out implement myself.

@dtolnay dtolnay changed the title Generic Error Variant Generic type parameter on error May 22, 2021
@Dessix
Copy link

Dessix commented Aug 3, 2021

I may consider accepting an attribute to place an additional bound on just the generated std::error::Error impl.

With #143, I believe that I've implemented the solution suggested in the above comment.

@mikevoronov
Copy link

mikevoronov commented Sep 20, 2021

Hi! Do you still have plan to support error(transparent) and #[from] for generics? I want to use it as TC like that

#[derive(Debug, ThisError)]
pub enum AVMError<E> {
    #[error(transparent)]
    DataStoreError(#[from] E),
}

Upd: Sorry, I've just found #149, but I still can't compile code like that

#[derive(Debug, ThisError)]
pub enum AVMError<E> {
    #[error(transparent)]
    RunnerError(#[from] RunnerError),

    #[error(transparent)]
    DataStoreError(#[from] E),
}

#[derive(Debug, ThisError)]
pub enum RunnerError {}

with a error

76 | #[derive(Debug, ThisError)]
   |                 ^^^^^^^^^
   |                 |
   |                 first implementation here
   |                 conflicting implementation for `errors::AVMError<errors::RunnerError>`

Are there any problems of having several error(transparent) at the same time?

Upd 2. Double sorry, I found out that it's generally impossible in Rust... :)

@Dessix
Copy link

Dessix commented Sep 20, 2021

Multiple transparent attributes are always supported; the conflict is due to generic From implementations because Rust lacks negative constraints on generics.

See this playground for an example based on your above code.

Honestly, if we can get that fixed in a reasonable, low-boilerplate way, the From and Into traits will become much more tolerable, but that's not part of thiserror, that's a Rust problem.

@mikevoronov
Copy link

@Dessix Thank you, yes, that what I meant by impossible in Rust. I wanted two #[from] where one for generic, then I tried to do that "by hands" without the macro, and realized that it's kinda constraint of current Rust type system.

Btw, I workarounded it with one #[from] for generic and map_err for the second error type...

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