Bring back slice::ref_slice as slice::from_ref. #45306

whitequark · 2017-10-15T13:31:55Z

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

rust-highfive · 2017-10-15T13:32:07Z

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 · 2017-10-15T13:33:30Z

I didn't do anything to bring back Option::as_slice and Result::as_slice because x.map_or(&[], ref_slice) is almost as short and just as easy to understand.

ghost · 2017-10-15T15:09:09Z

Maybe it's just me, but, looking at the name alone, it's hard to tell what slice::ref_slice actually does.
Perhaps slice::from_ref would sound better? Or slice::single_ref? 🚲

sfackler · 2017-10-18T03:17:26Z

I'm not particularly opposed to bringing this back, but I do agree with @stjepang's naming concern. We may want to land these as unstable initially, though I don't really know what good that would do in this specific case.

@rfcbot fcp merge

Mark-Simulacrum · 2017-10-18T03:26:12Z

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).

rfcbot · 2017-10-18T03:29:57Z

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 · 2017-10-18T03:30:31Z

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 · 2017-10-18T09:08:26Z

Since the naming is uncertain and insta-stable is proposed anyway, could they just be

impl<'a, T> From<&'a T> for &'a [T] { ... }
impl<'a, T> From<&'a mut T> for &'a mut [T] { ... }

whitequark · 2017-10-18T10:09:27Z

Personally, I think slice::from_ref is an obviously better name than slice::ref_slice. If I thought about naming at all (I did not, I just copied them from old rustc sources) then I would have chosen that too.

A From impl seems more disruptive to me.

alexcrichton · 2017-10-18T14:02:42Z

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 from_ref may be a bit nicer!

Ixrec · 2017-10-23T22:47:19Z

Also strongly agree with from_ref. My first guess as to what slice::ref_slice would do was take a slice of Ts and convert it to a slice of refs of T.

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 · 2017-10-23T22:54:43Z

Already renamed it, just not reflected in the PR.

carols10cents · 2017-10-30T16:36:06Z

ping @BurntSushi waiting for your ticky box here!

steveklabnik · 2017-10-30T17:26:36Z

My crate that provides these functions also provides an Option variant: https://github.com/steveklabnik/ref_slice/blob/master/src/lib.rs

I'm not sure if they're worth bringing in too, figured I'd bring it up.

whitequark · 2017-10-30T17:34:12Z

@steveklabnik #45306 (comment)

alexcrichton · 2017-10-31T22:13:44Z

I'm going to go ahead and check @BurntSushi's checkbox as this is just unstable anyway.

rfcbot · 2017-10-31T22:13:45Z

🔔 This is now entering its final comment period, as per the review above. 🔔

alexcrichton · 2017-10-31T22:13:55Z

@bors: r+

bors · 2017-10-31T22:13:56Z

📌 Commit 8431811 has been approved by alexcrichton

kennytm · 2017-11-01T05:16:13Z

src/libcore/slice/mod.rs

@@ -2450,6 +2450,22 @@ pub unsafe fn from_raw_parts_mut<'a, T>(p: *mut T, len: usize) -> &'a mut [T] {
    mem::transmute(Repr { data: p, len: len })
 }

+/// Converts a reference to T into a slice of length 1 (without copying).
+#[stable(feature = "from_ref", since = "1.22.0")]


I don't think we intend to insta-stable these?

kennytm · 2017-11-01T05:17:23Z

@bors r-

If the final FCP checkbox expects "this is just unstable anyway" the current code certainly isn't unstable.

alexcrichton · 2017-11-01T13:50:48Z

Oops sorry, thanks for catching that @kennytm!

whitequark · 2017-11-01T21:41:14Z

Can you please guide me on the stability markers here?

kennytm · 2017-11-01T21:49:11Z

@whitequark

File a tracking issue

Replace those #[stable] attributes with

#[unstable(feature = "from_ref", issue = "45678")]

Write an unstable-book entry in src/doc/unstable-book/src/library-features/from-ref.md if you like.

whitequark · 2017-11-01T21:55:09Z

Tracking issue filed as #45703, no unstable book entry since it seems far too trivial.

whitequark · 2017-11-01T21:57:01Z

Stability annotation updated.

kennytm · 2017-11-01T22:18:11Z

Thanks! It should be ready to go after fixing the build failure.

[00:02:34] error: use of unstable library feature 'from_ref' (see issue #45703)
[00:02:34]    --> /checkout/src/liballoc/slice.rs:123:23
[00:02:34]     |
[00:02:34] 123 | pub use core::slice::{from_ref, from_ref_mut};
[00:02:34]     |                       ^^^^^^^^
[00:02:34]     |
[00:02:34]     = help: add #![feature(from_ref)] to the crate attributes to enable
[00:02:34] 
[00:02:34] error: use of unstable library feature 'from_ref' (see issue #45703)
[00:02:34]    --> /checkout/src/liballoc/slice.rs:123:33
[00:02:34]     |
[00:02:34] 123 | pub use core::slice::{from_ref, from_ref_mut};
[00:02:34]     |                                 ^^^^^^^^^^^^
[00:02:34]     |
[00:02:34]     = help: add #![feature(from_ref)] to the crate attributes to enable

whitequark · 2017-11-01T22:21:46Z

Updated.

steveklabnik · 2017-11-01T22:48:17Z

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 · 2017-11-01T23:43:21Z

@bors r=alexcrichton

bors · 2017-11-01T23:43:22Z

📌 Commit 1cc88be has been approved by alexcrichton

bors · 2017-11-02T00:06:24Z

⌛ Testing commit 1cc88be with merge d5b69d4...

@nikomatsakis

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 · 2017-11-02T03:35:56Z

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

rust-highfive assigned sfackler Oct 15, 2017

Mark-Simulacrum added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Oct 18, 2017

rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Oct 18, 2017

aidanhs removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Oct 20, 2017

whitequark changed the title ~~Bring back slice::ref_slice.~~ Bring back slice::ref_slice as slice::from_ref. Oct 23, 2017

Bring back slice::ref_slice as slice::from_ref.

8431811

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.

carols10cents added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2017

rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Oct 31, 2017

rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Oct 31, 2017

kennytm requested changes Nov 1, 2017

View reviewed changes

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 1, 2017

whitequark mentioned this pull request Nov 1, 2017

Tracking issue for slice::from_ref and slice::from_ref_mut #45703

Closed

kennytm approved these changes Nov 1, 2017

View reviewed changes

De-stabilize core::slice::{from_ref, from_ref_mut}.

1cc88be

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 2, 2017

bors merged commit 1cc88be into rust-lang:master Nov 2, 2017

petrochenkov mentioned this pull request Nov 27, 2017

Move rustc_back modules where they belong. #46305

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bring back slice::ref_slice as slice::from_ref. #45306

Bring back slice::ref_slice as slice::from_ref. #45306

whitequark commented Oct 15, 2017 •

edited

Loading

rust-highfive commented Oct 15, 2017

whitequark commented Oct 15, 2017

ghost commented Oct 15, 2017

sfackler commented Oct 18, 2017

Mark-Simulacrum commented Oct 18, 2017

rfcbot commented Oct 18, 2017 •

edited by alexcrichton

Loading

sfackler commented Oct 18, 2017

scottmcm commented Oct 18, 2017

whitequark commented Oct 18, 2017

alexcrichton commented Oct 18, 2017

Ixrec commented Oct 23, 2017

whitequark commented Oct 23, 2017

carols10cents commented Oct 30, 2017

steveklabnik commented Oct 30, 2017

whitequark commented Oct 30, 2017

alexcrichton commented Oct 31, 2017

rfcbot commented Oct 31, 2017

alexcrichton commented Oct 31, 2017

bors commented Oct 31, 2017

kennytm Nov 1, 2017

kennytm commented Nov 1, 2017

alexcrichton commented Nov 1, 2017

whitequark commented Nov 1, 2017

kennytm commented Nov 1, 2017 •

edited

Loading

whitequark commented Nov 1, 2017

whitequark commented Nov 1, 2017

kennytm commented Nov 1, 2017

whitequark commented Nov 1, 2017

steveklabnik commented Nov 1, 2017 via email

kennytm commented Nov 1, 2017

bors commented Nov 1, 2017

bors commented Nov 2, 2017

bors commented Nov 2, 2017

Bring back slice::ref_slice as slice::from_ref. #45306

Bring back slice::ref_slice as slice::from_ref. #45306

Conversation

whitequark commented Oct 15, 2017 • edited Loading

rust-highfive commented Oct 15, 2017

whitequark commented Oct 15, 2017

ghost commented Oct 15, 2017

sfackler commented Oct 18, 2017

Mark-Simulacrum commented Oct 18, 2017

rfcbot commented Oct 18, 2017 • edited by alexcrichton Loading

sfackler commented Oct 18, 2017

scottmcm commented Oct 18, 2017

whitequark commented Oct 18, 2017

alexcrichton commented Oct 18, 2017

Ixrec commented Oct 23, 2017

whitequark commented Oct 23, 2017

carols10cents commented Oct 30, 2017

steveklabnik commented Oct 30, 2017

whitequark commented Oct 30, 2017

alexcrichton commented Oct 31, 2017

rfcbot commented Oct 31, 2017

alexcrichton commented Oct 31, 2017

bors commented Oct 31, 2017

kennytm Nov 1, 2017

Choose a reason for hiding this comment

kennytm commented Nov 1, 2017

alexcrichton commented Nov 1, 2017

whitequark commented Nov 1, 2017

kennytm commented Nov 1, 2017 • edited Loading

whitequark commented Nov 1, 2017

whitequark commented Nov 1, 2017

kennytm commented Nov 1, 2017

whitequark commented Nov 1, 2017

steveklabnik commented Nov 1, 2017 via email

kennytm commented Nov 1, 2017

bors commented Nov 1, 2017

bors commented Nov 2, 2017

bors commented Nov 2, 2017

whitequark commented Oct 15, 2017 •

edited

Loading

rfcbot commented Oct 18, 2017 •

edited by alexcrichton

Loading

kennytm commented Nov 1, 2017 •

edited

Loading