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

Provide implementations of From<&str> and From<String> for Report #60

Closed
faulesocke opened this issue Oct 19, 2021 · 4 comments
Closed

Comments

@faulesocke
Copy link

faulesocke commented Oct 19, 2021

The standard library has the implementations From<'_ &str> for Box<dyn Error> and From<String> for Box<dyn Error>. This allows for example the following, convenient code:

fn foo() -> Result<i32, Box<dyn Error>> {
    let opt: Option<i32> = None;
    opt.ok_or("The Option was None");
}

assert!(foo().is_err());

Basically I use it a lot to change Options into Results in application code. When using eyre as a drop-in replacement for Result<..., Box<dyn Error>> I have to change all of these ok_or calls and wrap the strings with an extra macro call to eyre!. This is inconvenient. Therefore this is a request to provide these implementations.

References:

@yaahc
Copy link
Collaborator

yaahc commented Oct 19, 2021

These impls are only valid because they exist in the same crate that defines Error. If we attempt to add the same impl in eyre it will give us an error indicating potential overlap with From<E: Error> for Report indicating that &str and String could introduce Error impls in the future. This is one of the primary issues blocking moving the Error trait into core and we're currently working on fixing it by adding support for negative trait impls to the coherence engine.

Once that initiative has completed we should be able to add impl !Error for &str and impl !Error for String and then add the relevant From impls to eyre.

@faulesocke
Copy link
Author

Oh interesting, didn't knew that. But wouldn't it be easier to just impl Error for String and impl Error for &str in the stdlib?

@yaahc
Copy link
Collaborator

yaahc commented Oct 25, 2021

Oh interesting, didn't knew that. But wouldn't it be easier to just impl Error for String and impl Error for &str in the stdlib?

That's already been ruled out. They absolutely could impl Error, but because they do not always semantically represent an error it would be inappropriate to do so.

@ten3roberts
Copy link
Contributor

I am closing this issue as the points seem to be resolved as a string alone is not necessarily an error but may be any sort of thing, and as such making it an error would be errorprone

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

No branches or pull requests

3 participants