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

Convert Intos to Froms. #42129

Closed
wants to merge 1 commit into from
Closed

Conversation

clarfonthey
Copy link
Contributor

This didn't work before but supposedly this should work now. It's better to have From impls over Into.

@rust-highfive
Copy link
Collaborator

r? @BurntSushi

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

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 20, 2017
@Mark-Simulacrum
Copy link
Member

Travis failed:

[00:02:05] error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g. `MyStruct<T>`); only traits defined in the current crate can be implemented for a type parameter
[00:02:05]     --> /checkout/src/libcollections/vec.rs:2128:1
[00:02:05]      |
[00:02:05] 2128 | / impl<T> From<Vec<T>> for Box<[T]> {
[00:02:05] 2129 | |     fn from(v: Vec<T>) -> Box<[T]> {
[00:02:05] 2130 | |         v.into_boxed_slice()
[00:02:05] 2131 | |     }
[00:02:05] 2132 | | }
[00:02:05]      | |_^
[00:02:05]
[00:02:05] error: aborting due to previous error

@BurntSushi
Copy link
Member

In principle this seems fine to me, if you can get it to work. However, are there any backward compatibility/inference failure hazards? Should this get a crater run?

@BurntSushi BurntSushi added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 21, 2017
@clarfonthey
Copy link
Contributor Author

@BurntSushi doesn't require crater run because From automatically implements Into, so, in principle this is just adding a From impl

@BurntSushi
Copy link
Member

Right. I understand it shouldn't. But I've been fooled by things like this in the past. :-)

@alexcrichton
Copy link
Member

Ah yeah unfortunately these impls are not possible due to coherence. In that case I'm going to close this.

@ollie27
Copy link
Member

ollie27 commented May 25, 2017

I believe it's only impl<T> From<Vec<T>> for Box<[T]> which doesn't work. The others compiled fine when I tried them.

@alexcrichton
Copy link
Member

Ah ok! Want to send a PR with those implementations?

@ollie27
Copy link
Member

ollie27 commented May 25, 2017

Sure: #42227

@clarfonthey clarfonthey deleted the into_to_from branch May 27, 2017 19:27
@clarfonthey
Copy link
Contributor Author

Thanks @ollie27 for beating me to it

bors added a commit that referenced this pull request Jun 4, 2017
Convert Intos to Froms.

This is a resubmission of #42129 without `impl<T> From<Vec<T>> for Box<[T]>`.
bors added a commit that referenced this pull request Jun 8, 2017
Convert Intos to Froms.

This is a resubmission of #42129 without `impl<T> From<Vec<T>> for Box<[T]>`.
bors added a commit that referenced this pull request Jun 17, 2017
Convert `Into<Box<[T]>> for Vec<T>` into `From<Vec<T>> for Box<[T]>`

As the `collections` crate has been merged into `alloc` in #42648 this impl is now possible. This is the final part of #42129 missing from #42227.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 20, 2017
Convert `Into<Box<[T]>> for Vec<T>` into `From<Vec<T>> for Box<[T]>`

As the `collections` crate has been merged into `alloc` in rust-lang#42648 this impl is now possible. This is the final part of rust-lang#42129 missing from rust-lang#42227.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 4, 2017
Convert Intos to Froms.

This is a resubmission of rust-lang#42129 without `impl<T> From<Vec<T>> for Box<[T]>`.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 4, 2017
Convert Intos to Froms.

This is a resubmission of rust-lang#42129 without `impl<T> From<Vec<T>> for Box<[T]>`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants