Move the <dyn Error>::chain() method to a trait #69163
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @withoutboats |
|
Don't you have to import the trait then? Wouldn't just a free standing |
Hmm... works, but not so ergonomic. E.g. for |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Because `chain()` was only implemented for `dyn Error`, users had
to cast errors to a `&(dyn Error)` to use the `chain()` method.
```rust
let mut iter = (&my_error as &(dyn Error)).chain();
// or
let mut iter = <dyn Error>::chain(&my_error);
// or
let mut iter = Error::chain(&my_error);
```
What I would liked to have is
```rust
pub trait Error: Debug + Display {
fn source(&self) -> Option<&(dyn Error + 'static)> {
None
}
fn chain(&self) -> Chain<'_> where Self: Sized + 'static {
Chain {
current: Some(self),
}
}
}
impl dyn Error + 'static {
fn chain(&self) -> Chain<'_> {
Chain {
current: Some(self),
}
}
}
```
This doesn't work, because of:
rust-lang#69161
This patch introduces an ErrorChain trait, which accomplishes the
job and is implemented for `Error` and `dyn Error + 'static`.
|
I think it's very important to have chain as a method method call. Otherwise it will not be very useful to add more functionality to the Chain, because creating a Chain will require too much effort. So I fully support the idea of this PR. I would very much prefer #69161 to be fixed though, since I think it's much nicer not to have to import a separate trait to get access to this method. |
|
@haraldh I figured out a way to get this to work, without requiring the ErrorChain trait to be added to the scope: It does trigger the warning added in #51443, but I guess it might be fine in this case. If not it seems reasonable to use this in nightly until #69161 is fixed. |
cool, but I don't want everybody using |
|
I would expect this can be done by adding |
If it doesn't work in the playground, I doubt it works without global disable. |
|
r? @dtolnay |
dtolnay
reviewed
Mar 29, 2020
There was a problem hiding this comment.
Thanks for the PR!
I am interested in having a chain method that works better, but I would prefer not to add a new standard library trait just for this. I would instead like to see work on #69161 and then implement chain in the way you say you want in the PR description. A trait like this would only hamstring us once the method resolution issue is fixed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because
chain()was only implemented fordyn Error, users hadto cast errors to a
&(dyn Error)to use thechain()method.What I would liked to have is
This doesn't work, because of:
#69161
This patch introduces an ErrorChain trait, which accomplishes the
job and is implemented for
Erroranddyn Error + 'static.