Uh oh!
There was an error while loading. Please reload this page.
Bring back slice::ref_slice as slice::from_ref. - #45306
Conversation
rust-highfive
commented
Oct 15, 2017
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
whitequark
commented
Oct 15, 2017
I didn't do anything to bring back |
ghost
commented
Oct 15, 2017
Maybe it's just me, but, looking at the name alone, it's hard to tell what |
sfackler
commented
Oct 18, 2017
Mark-Simulacrum
commented
Oct 18, 2017
I've added the waiting-on-crater label for now so that if we decide to go ahead with this we do a crater run -- I think it's worth doing that just in case others have implemented ref_slice (which could play into the naming bikeshed, too, so probably best after fcp). |
Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
sfackler
commented
Oct 18, 2017
These are free functions, so I don't think there's as much of a concern with collisions as you'd have with new methods. |
scottmcm
commented
Oct 18, 2017
Since the naming is uncertain and insta-stable is proposed anyway, could they just be impl<'a,T>From<&'aT>for&'a[T]{ ... }impl<'a,T>From<&'amutT>for&'amut[T]{ ... } |
whitequark
commented
Oct 18, 2017
Personally, I think A |
alexcrichton
commented
Oct 18, 2017
I'd personally prefer to initially add these as unstable (but perhaps push on stabilization soon after landing), and I agree with @whitequark that a name like |
Ixrec
commented
Oct 23, 2017
Also strongly agree with |
These functions were deprecated and removed in 1.5, but such simple functionality shouldn't require using unsafe code, and it isn't cluttering libstd too much.
whitequark
commented
Oct 23, 2017
Already renamed it, just not reflected in the PR. |
carols10cents
commented
Oct 30, 2017
ping @BurntSushi waiting for your ticky box here! |
steveklabnik
commented
Oct 30, 2017
My crate that provides these functions also provides an I'm not sure if they're worth bringing in too, figured I'd bring it up. |
whitequark
commented
Oct 30, 2017
alexcrichton
commented
Oct 31, 2017
I'm going to go ahead and check @BurntSushi's checkbox as this is just unstable anyway. |
rfcbot
commented
Oct 31, 2017
🔔 This is now entering its final comment period, as per the review above. 🔔 |
alexcrichton
commented
Oct 31, 2017
@bors: r+ |
bors
commented
Oct 31, 2017
📌 Commit 8431811 has been approved by |
| } | ||
| /// Converts a reference to T into a slice of length 1 (without copying). | ||
| #[stable(feature = "from_ref", since = "1.22.0")] |
There was a problem hiding this comment.
I don't think we intend to insta-stable these?
kennytm
commented
Nov 1, 2017
@bors r- If the final FCP checkbox expects "this is just unstable anyway" the current code certainly isn't unstable. |
alexcrichton
commented
Nov 1, 2017
Oops sorry, thanks for catching that @kennytm! |
whitequark
commented
Nov 1, 2017
Can you please guide me on the stability markers here? |
|
whitequark
commented
Nov 1, 2017
Tracking issue filed as #45703, no unstable book entry since it seems far too trivial. |
whitequark
commented
Nov 1, 2017
Stability annotation updated. |
kennytm
commented
Nov 1, 2017
Thanks! It should be ready to go after fixing the build failure. |
whitequark
commented
Nov 1, 2017
Updated. |
steveklabnik
commented
Nov 1, 2017
via email
Library stuff doesn't need an unstable book entry as the api docs are docs;
the unstable book pages get generated automatically. …On Wed, Nov 1, 2017 at 6:22 PM, whitequark ***@***.***> wrote:
Updated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#45306 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABsikXJgsaCpcQfEEdjztR6V-nnUeB7ks5syO8KgaJpZM4P5v2u>
.
|
kennytm
commented
Nov 1, 2017
@bors r=alexcrichton |
bors
commented
Nov 1, 2017
📌 Commit 1cc88be has been approved by |
bors
commented
Nov 2, 2017
Bring back slice::ref_slice as slice::from_ref. These functions were deprecated and removed in 1.5, but such simple functionality shouldn't require using unsafe code, and it isn't cluttering libstd too much. The original removal was quite contentious (see #27774), since then we've had precedent for including such nuggets of functionality (see rust-lang/rfcs#1789), and @nikomatsakis has provided a lot of use cases in rust-lang/rfcs#1789 (comment). Hence this PR. I'm not too sure what to do with stability, feel free to correct me. It seems pointless to go through stabilization for these functions though. cc @aturon
bors
commented
Nov 2, 2017
☀️ Test successful - status-appveyor, status-travis |
These functions were deprecated and removed in 1.5, but such simple
functionality shouldn't require using unsafe code, and it isn't
cluttering libstd too much.
The original removal was quite contentious (see #27774), since then
we've had precedent for including such nuggets of functionality (see rust-lang/rfcs#1789),
and @nikomatsakis has provided a lot of use cases in rust-lang/rfcs#1789 (comment).
Hence this PR.
I'm not too sure what to do with stability, feel free to correct me.
It seems pointless to go through stabilization for these functions though.
cc @aturon