Skip to content

std: impl From<String> for Box<Error + Send> #23979

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

Merged
merged 1 commit into from
Apr 4, 2015

Conversation

Ryman
Copy link
Contributor

@Ryman Ryman commented Apr 2, 2015

No description provided.

@rust-highfive
Copy link
Contributor

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@reem
Copy link
Contributor

reem commented Apr 2, 2015

I've wanted this too, but if we're going to do this we should probably just take the clearer step conceptually of just implementing Error for String.

@alexcrichton
Copy link
Member

@reem hm I'm a little confused, this intentionally does not implement Error for &str or String, just From into a Box<Error+Send>. Could you elaborate on what you're thinking a bit?

@reem
Copy link
Contributor

reem commented Apr 2, 2015

@alexcrichton this feels like a weird half-step to me and has some drawbacks. For instance, this doesn't expose the exact underlying type of the error and can't be used with traits that inherit from Error, e.g. rust-error.

@reem
Copy link
Contributor

reem commented Apr 2, 2015

I'm saying that I think that implementing Error for String would be fine, and have more use cases than just this implementation.

@alexcrichton
Copy link
Member

I personally feel that Error for String is too ambitious and would rather have a clear opt-in type for a "string-like error". I like these impls as-is as you have to explicitly ask for a string to be interpreted as an error. I also think that not exposing the underlying type is a good thing as it's one less thing to stabilize for now and it's possible to add it later if necessary.

@aturon
Copy link
Member

aturon commented Apr 3, 2015

Hm, I'm torn on the question of making String implement Error. My instinct, like @alexcrichton, is that Error is a meaningful designation that should only be applied to types that are specifically intended to represent errors, and that Into<Box<Error>> is a nice way of saying "can be interpreted as an error". But I'm struggling to say why, exactly, this seems important.

FWIW, taking the step here does not preclude us from making String: Error in the future, so I think regardless we should follow this conservative approach for now, and see how the idioms evolve over time before taking the next step.

@alexcrichton
Copy link
Member

I agree with @aturon in that it should be backwards-compatible to add the impl (thanks to new coherence changes!), so it should be ok to take this step for now.

@bors: r+ 8b719ee

@lilyball
Copy link
Contributor

lilyball commented Apr 3, 2015

I agree with @alexcrichton and @aturon as well, that a separate distinct type is appropriate and that we probably don't want to make String: Error.

@bors
Copy link
Collaborator

bors commented Apr 4, 2015

⌛ Testing commit 8b719ee with merge bcae782...

@bors
Copy link
Collaborator

bors commented Apr 4, 2015

@bors bors merged commit 8b719ee into rust-lang:master Apr 4, 2015
Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 4, 2015
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.

8 participants