Skip to content

std: Stabilize a number of small APIs #27370

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
Jul 31, 2015

Conversation

alexcrichton
Copy link
Member

The following APIs were all marked with a #[stable] tag:

  • process::Child::id
  • error::Error::is
  • error::Error::downcast
  • error::Error::downcast_ref
  • error::Error::downcast_mut
  • io::Error::get_ref
  • io::Error::get_mut
  • io::Error::into_inner
  • hash::Hash::hash_slice
  • hash::Hasher::write_{i,u}{8,16,32,64,size}

The following APIs were all marked with a `#[stable]` tag:

* process::Child::id
* error::Error::is
* error::Error::downcast
* error::Error::downcast_ref
* error::Error::downcast_mut
* io::Error::get_ref
* io::Error::get_mut
* io::Error::into_inner
* hash::Hash::hash_slice
* hash::Hasher::write_{i,u}{8,16,32,64,size}
@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 28, 2015
@alexcrichton
Copy link
Member Author

r? @aturon

@rust-highfive rust-highfive assigned aturon and unassigned huonw Jul 28, 2015
@rust-highfive
Copy link
Contributor

r? @huonw

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

@alexcrichton alexcrichton added the I-needs-decision Issue: In need of a decision. label Jul 28, 2015
@@ -233,8 +232,7 @@ impl Error {
///
/// If this `Error` was constructed via `new` then this function will
/// return `Some`, otherwise it will return `None`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a note to the docs for these methods about the need to call via UFCS - it confused the hell out of me for a while when I was writing tests for them.

EDIT: Ah, looks like it works now from the tests, nvm.

@sfackler
Copy link
Member

lgtm

@brson
Copy link
Contributor

brson commented Jul 28, 2015

Is there any paper trail documenting the rationale for stabilizing these? Why these and why now?

@alexcrichton
Copy link
Member Author

We've got a huge number (~150) of unstable features in the standard library, and we need to start taking action on them. Many of them have been around for months so they either need to be deprecated or they need to be stabilized. This is a small set of features which feel pretty uncontroversial and are part of the crucial designs of primitives leading up to 1.0:

  • process::Child::id - otherwise there is no way to get the pid of a process
  • error::Error::{is, downcast*} - this is a vital part of the design of the Error trait being able to carry arbitrary payloads. This must be stabilized in one form eventually, and the current form seems quite adequate.
  • io::Error::{get_ref, get_mut, into_inner} - another crucial part of the design of io::Error, this will need to be stabilized eventually and we just didn't do so as they were added recently before 1.0 was released
  • hash::Hash::hash_slice - this is necessary for high-performance hashing of &[u8] and there's no particular reason we should prevent that optimization being implemented for user-defined types
  • hash::Hasher::write_* - this is necessary for high-performance hashing (not always working through an array of bytes), and there's no reason to limit that to in-tree implementations.

I would like to include more but they all seemed to have at least one unresolved question.

@Gankra
Copy link
Contributor

Gankra commented Jul 29, 2015

Don't care about process and io, but I recall we historically had issues with the hasher API as is. Are these parts not part of the problem, or has something changed? (maybe I'm misremembering)

@alexcrichton
Copy link
Member Author

You can find more of the initial discussion here: #19673, but it was largely about the high-level design (where generics are, the signature of hash, etc). I don't believe there were any holdouts of the extra methods being stabilized here other than "we want to make sure this isn't too onerous".

@Gankra
Copy link
Contributor

Gankra commented Jul 29, 2015

👍 on the hasher stuff -- seems harmless at worst.

@brson brson added the relnotes Marks issues that should be documented in the release notes of the next release. label Jul 29, 2015
@brson
Copy link
Contributor

brson commented Jul 29, 2015

Thanks for the explanation.

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 29, 2015

📌 Commit 76db37e has been approved by brson

@bors
Copy link
Collaborator

bors commented Jul 30, 2015

⌛ Testing commit 76db37e with merge 8c6e940...

@bors
Copy link
Collaborator

bors commented Jul 30, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton alexcrichton removed the I-needs-decision Issue: In need of a decision. label Jul 30, 2015
@alexcrichton
Copy link
Member Author

@bors: retry

On Thu, Jul 30, 2015 at 3:25 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-mac-32-opt
http://buildbot.rust-lang.org/builders/auto-mac-32-opt/builds/5947


Reply to this email directly or view it on GitHub
#27370 (comment).

@bors
Copy link
Collaborator

bors commented Jul 30, 2015

⌛ Testing commit 76db37e with merge 938f5d7...

@bors
Copy link
Collaborator

bors commented Jul 30, 2015

💔 Test failed - auto-linux-64-opt

@alexcrichton
Copy link
Member Author

@bors: retry

On Thu, Jul 30, 2015 at 11:42 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-64-opt
http://buildbot.rust-lang.org/builders/auto-linux-64-opt/builds/5902


Reply to this email directly or view it on GitHub
#27370 (comment).

@bors
Copy link
Collaborator

bors commented Jul 30, 2015

⌛ Testing commit 76db37e with merge 70fe4ef...

@bors
Copy link
Collaborator

bors commented Jul 30, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton
Copy link
Member Author

@bors: retry

On Thu, Jul 30, 2015 at 2:25 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-mac-32-opt
http://buildbot.rust-lang.org/builders/auto-mac-32-opt/builds/5955


Reply to this email directly or view it on GitHub
#27370 (comment).

bors added a commit that referenced this pull request Jul 31, 2015
The following APIs were all marked with a `#[stable]` tag:

* process::Child::id
* error::Error::is
* error::Error::downcast
* error::Error::downcast_ref
* error::Error::downcast_mut
* io::Error::get_ref
* io::Error::get_mut
* io::Error::into_inner
* hash::Hash::hash_slice
* hash::Hasher::write_{i,u}{8,16,32,64,size}
@bors
Copy link
Collaborator

bors commented Jul 31, 2015

⌛ Testing commit 76db37e with merge cb250b7...

@bors bors merged commit 76db37e into rust-lang:master Jul 31, 2015
@alexcrichton alexcrichton deleted the stabilize-easy branch August 17, 2015 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. 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.

8 participants