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

Stabilize library features for 1.18.0 #41904

Merged
merged 1 commit into from
May 22, 2017

Conversation

sfackler
Copy link
Member

@sfackler sfackler commented May 11, 2017

Closes #38863
Closes #38980
Closes #38903
Closes #36648

r? @alexcrichton

@rust-lang/libs

@sfackler sfackler added beta-nominated Nominated for backporting to the compiler in the beta channel. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels May 11, 2017
@WiSaGaN
Copy link
Contributor

WiSaGaN commented May 11, 2017

I think you mean "Closes #33417" instead of "Closes #34417"?

@sfackler
Copy link
Member Author

Fixed, thanks

@alexcrichton alexcrichton added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 11, 2017
@alexcrichton
Copy link
Member

Looks like some doc building failed? Other than that r=me, thanks @sfackler!

@ollie27
Copy link
Member

ollie27 commented May 11, 2017

I don't agree with stabilising TryFrom without impl<T, U> TryFrom<U> for T where T: From<U>. However if it really needs to be stabilised now for whatever reason then it should at least be properly documented. I'd like to see the documentation explaining at least the following:

  1. Which types should get an impl of the form TryFrom<T> for T? For example, why is there TryFrom<u8> for u8 but not TryFrom<char> for char?
  2. Which From<U> for T impls should get a TryFrom<U> for T impl. For exmple, why is there TryFrom<u8> for u32 but no TryFrom<u8> for char?
  3. Do the error types for converting to a certain type have to be the same? That was implied by several comments in Tracking issue for TryFrom/TryInto traits #33417 but TryFrom<u8> for u32 and TryFrom<&str> for u32 have different error types.

Some example impls and uses would also be nice.

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

@ollie27 If people want to wait for however many months or years it takes for ! to stabilize before landing TryFrom, then so be it, I've removed it from this PR - I'm done dealing with that mess. It's not even clear to me at this point that a single trait makes sense for all of the things we're asking of it. I'm frankly starting to think we should just deprecate and remove the whole thing.

@alexcrichton
Copy link
Member

@sfackler it looks like tidy failed here, but I'm ok handling TryFrom elsewhere if you'd like. Want to fixup tidy and r=me?

@Mark-Simulacrum
Copy link
Member

Looks like this is ready for another review; I think @sfackler removed TryFrom with ! as the error but I'm not really sure...

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 20, 2017
@sfackler
Copy link
Member Author

I just hopefully fixed tidy

@Mark-Simulacrum
Copy link
Member

And the mighty tidy says ... no:

[00:02:25] tidy error: The Unstable Book has a 'library feature' section 'try_from' which doesn't correspond to an unstable library feature
[00:02:25] some tidy checks failed
[00:02:25] 
[00:02:25] 
[00:02:25] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/tidy" "/checkout/src" "--no-vendor"
[00:02:25] expected success, got: exit code: 1
[00:02:25] 

@sfackler
Copy link
Member Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented May 21, 2017

📌 Commit 61ce6a4 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented May 21, 2017

⌛ Testing commit 61ce6a4 with merge aa5b6fc...

@bors
Copy link
Contributor

bors commented May 21, 2017

💔 Test failed - status-travis

@Mark-Simulacrum
Copy link
Member

[01:01:34] failures:
[01:01:34]     collections/hash/map.rs - collections::hash::map::HashMap<K, V, S>::retain (line 1233)
[01:01:34]     collections/hash/set.rs - collections::hash::set::HashSet<T, S>::retain (line 660)
[01:01:34]     net/tcp.rs - net::tcp::TcpStream::peek (line 345)
[01:01:34]     net/udp.rs - net::udp::UdpSocket::peek (line 640)
[01:01:34]     net/udp.rs - net::udp::UdpSocket::peek_from (line 114)

@sfackler
Copy link
Member Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented May 21, 2017

📌 Commit 7c2cd93 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented May 21, 2017

⌛ Testing commit 7c2cd93 with merge f6cc40f...

bors added a commit that referenced this pull request May 21, 2017
Stabilize library features for 1.18.0

Closes #38863
Closes #38980
Closes #38903
Closes #36648

r? @alexcrichton

@rust-lang/libs
@bors
Copy link
Contributor

bors commented May 22, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing f6cc40f to master...

@bors bors merged commit 7c2cd93 into rust-lang:master May 22, 2017
@sfackler sfackler deleted the 1.18-stabilization branch May 22, 2017 01:12
@brson brson mentioned this pull request May 23, 2017
bors added a commit that referenced this pull request May 23, 2017
[beta] backports

- #42006
- #41904

Bumps the version so we get a new beta.

@sfackler libs backports here

@nikomatsakis there are several other nominated PRs that don't cherry-pick cleanly: https://github.com/rust-lang/rust/pulls?q=is%3Apr+label%3Abeta-nominated+is%3Aclosed. Can you take a look or recruit someone else to?

r? @alexcrichton
@brson brson removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

7 participants