-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Leftovers from #39594; From<Box> impls #40009
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
448896b
to
321233e
Compare
Discussed during libs triage today the conclusion was that this looks good to go, but could the |
@alexcrichton It can, although both |
Yes let's move to a separate PR |
I separated out the |
54f1a9c
to
416e25f
Compare
92f4ac6
to
9a6fc62
Compare
And I've finished the impls! Should work properly; waiting on Travis. |
Apparently, Travis is failing because of a type mismatch that doesn't seem to exist? I'm going to test it out locally and see if I can fix it. |
So, this seems pretty broken:
In what circumstances aren't those the same type? Another note: the implementations for those types do use the types they're asking for, which makes no sense to me. |
Got some help on IRC from @thepowersgang (I think); apparently, this is causing the error:
So right now I disabled the Box -> String and Box -> Vec conversions on tests. Shouldn't be too bad, considering how they just call an already-tested function. |
e092016
to
2b8b7e3
Compare
This is ready to merge now, @alexcrichton. |
Looks like there may be travis errors? |
This seems to have also grown impls since last we looked? The I'd be uncomfortable r+'ing all the new impls, so I'll tag with T-libs again. |
For Travis, it looks like the errors are from a lack of I perfectly understand the caution on the new impls; will be waiting for a response from libs. :) |
Discussed during triage today, conclusion was that these were good to go. @bors: r+ |
📌 Commit 560944b has been approved by |
Leftovers from #39594; From<Box> impls These are a few more impls that follow the same reasoning as those from #39594. What's included: * `From<Box<str>> for String` * `From<Box<[T]>> for Vec<T>` * `From<Box<CStr>> for CString` * `From<Box<OsStr>> for OsString` * `From<Box<Path>> for PathBuf` * `Into<Box<str>> for String` * `Into<Box<[T]>> for Vec<T>` * `Into<Box<CStr>> for CString` * `Into<Box<OsStr>> for OsString` * `Into<Box<Path>> for PathBuf` * `<Box<CStr>>::into_c_string` * `<Box<OsStr>>::into_os_string` * `<Box<Path>>::into_path_buf` * Tracking issue for latter three methods + three from previous PR. Currently, the opposite direction isn't doable with `From` (only `Into`) because of the separation between `liballoc` and `libcollections`. I'm holding off on those for a later PR.
☀️ Test successful - status-appveyor, status-travis |
These are a few more impls that follow the same reasoning as those from #39594.
What's included:
From<Box<str>> for String
From<Box<[T]>> for Vec<T>
From<Box<CStr>> for CString
From<Box<OsStr>> for OsString
From<Box<Path>> for PathBuf
Into<Box<str>> for String
Into<Box<[T]>> for Vec<T>
Into<Box<CStr>> for CString
Into<Box<OsStr>> for OsString
Into<Box<Path>> for PathBuf
<Box<CStr>>::into_c_string
<Box<OsStr>>::into_os_string
<Box<Path>>::into_path_buf
Currently, the opposite direction isn't doable with
From
(onlyInto
) because of the separation betweenliballoc
andlibcollections
. I'm holding off on those for a later PR.