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

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

Closed
whitequark opened this Issue Nov 1, 2017 · 19 comments

Comments

Projects
None yet
9 participants
@whitequark
Copy link
Member

whitequark commented Nov 1, 2017

Tracking issue for feature from_ref

// std::slice
pub fn from_ref<T>(s: &T) -> &[T];
pub fn from_ref_mut<T>(s: &mut T) -> &mut [T];
@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Nov 15, 2017

For archeology purpose: these were added to the standard library in 2014 419ac4a, then deprecated and moved to crates.io in 2015: #27774.

Now that we’ve decided to bring them back and are going through the process motions, can we start FCP to stabilize? CC @rust-lang/libs

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented Nov 15, 2017

Is there anything else in the standard library that uses "ref_mut" to refer to &mut T? In RFC 199 we went the opposite way.

Is it intended to be understood as (from_ref)_mut or from_(ref_mut)?

slice::from_mut wouldn't seem accurate to me but is there a different naming that works better with _ref and _mut suffixes?

@whitequark

This comment has been minimized.

Copy link
Member Author

whitequark commented Nov 15, 2017

Is it intended to be understood as (from_ref)mut or from(ref_mut)?

The latter. I think slice::from_mut is better here, too. I did not realize this in my initial PR.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Nov 15, 2017

I had left a brief comment in the original PR, but I didn't see anyone reply: my crate has two extra Option based versions as well, I don't know if we want to pull them in, or if my crate will still have some small use for those variants.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Nov 15, 2017

opt_slice(x) can be written as x.as_ref().map(ref_slice) which does not use unsafe (and is fairly standard Option manipulation code). So I think the need to include it is less pressing.

@whitequark

This comment has been minimized.

Copy link
Member Author

whitequark commented Nov 16, 2017

@steveklabnik I replied to you: #45306 (comment)

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Nov 16, 2017

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 17, 2018

Looks good to me to stabilize.

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Mar 17, 2018

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), 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.

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented Mar 17, 2018

Given that we have Option::get_ref and Option::get_mut which is not called get_ref_mut, I think here it should be from_ref and from_mut rather than from_ref_mut.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 17, 2018

Makes sense. Let’s say stabilize with that change.

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Apr 18, 2018

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

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Apr 28, 2018

The final comment period is now complete.

kennytm added a commit to kennytm/rust that referenced this issue May 22, 2018

Rollup merge of rust-lang#50945 - stjepang:stabilize-from-ref, r=Simo…
…nSapin

Stabilize feature from_ref

Function `from_ref_mut` is now renamed to `from_mut`, as discussed in rust-lang#45703.

Closes rust-lang#45703.

r? @SimonSapin

@bors bors closed this in #50945 May 22, 2018

@stjepang

This comment has been minimized.

Copy link
Contributor

stjepang commented Jul 27, 2018

Just one more last-minute question before stabilization: Why does slice::from_ref return a &[T]?

Wouldn't it be more useful to return a &[T; 1], which can be coerced into &[T]?
Are we going to add such a function later as array::from_ref?

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jul 27, 2018

That’s… a good point.

Though as you say a function that returns &[T; 1] would belong more in std::array than std::slice. If we add it there, should we also remove it from std::slice (because you can get the same with coercion) or should we keep both?

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jul 27, 2018

Yet another alternative might be From impls, instead of free functions.

@Kimundi

This comment has been minimized.

Copy link
Member

Kimundi commented Jul 27, 2018

I could image that implementing this with From impls might have negative inference or derfer-coercion effects, since passing a &Vec<T> for example might end in a &[Vec<T>] instead of a &[T]

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jul 27, 2018

Right, adding another x.into() for every x: &T would probably make inference impractical.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 27, 2018

An excellent point @stjepang! It's a bit close to the release at this point, and I think @SimonSapin has a good idea of putting those in std::array as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.