Introduce Option::<&T>::borrowed

[nox]

No description provided.

[BurntSushi]

Rendered.

[sfackler]

I'm not sure about Borrow vs Deref and naming, but I've definitely run into this, and it's kind of annoying/ugly to have to do the current foo.map(|s| &**s) dance. We'd probably want this on Iterator as well?

[BurntSushi]

Yeah, I know I've at least burned x.as_ref().map(|x| &**x) into my brain as well. At this point, I barely even notice it, but taking a step back, I can see how unintuitive it might be.

One thing I worry about is the overlap between as_ref and borrowed. As a thought experiment, how would you explain the difference between them? (Keeping in mind that as_ref in this context is a concrete method on Option and not part of the AsRef trait.)

[sfackler]

I believe that the only difference is that borrowed is required to be consistent wrt Hash/Eq/Ord, but yeah, the number of traits that do basically the same thing here is not great.

[bluss]

I would like to focus on AsRef, and not spread Borrow too far away from its use case (which was originally HashMap::get, BTreeMap::get).

[sfackler]

That seems reasonable, though naming will be a bit less obvious (please no as_refed!).

[eddyb]

There's probably some connection with #1403, as people have mentioned AsRef coercions recently.

[nox]

I would like to focus on AsRef, and not spread Borrow too far away from its use case (which was originally HashMap::get, BTreeMap::get).

Isn't that already a lost battle, with ToOwned and Cow? And neither HashMap nor BTreeMap need BorrowMut, right? I would rather make AsRef less used.

[nox]

The book says:

Choose Borrow when you want to abstract over different kinds of borrowing, or when you’re building a data structure that treats owned and borrowed values in equivalent ways, such as hashing and comparison.

Choose AsRef when you want to convert something to a reference directly, and you’re writing generic code.

So I would rather prefer we use Borrow for this.

[bluss]

Borrow is constrained by its hash/ordering guarantees, and those have no special connection to this use case. AsRef is more general and versatile, supporting str → Path, String → [u8] and so on.

[steveklabnik]

I also believe that this would be AsRef; @aturon and I had a lot of conversations about asref/borrow in the past...

[alexcrichton]

I agree with @bluss that we likely want to leverage AsRef here instead of Borrow to allow for other conversions like Option<Cow<str>> to Option<&str> and such. I'm not entirely sure what the method would be however, as I also agree with @sfackler that as_refed is pretty bad.

Also the RFC motivation may want to be updated slightly, the stated problem is:

How to convert from Option<String> to Option<&str> is sometimes asked

Yet the RFC does not solve this problem as it defines a method on Option<&'a T>. Instead, I would have expected the signature:

impl<T> Option<T> {
    fn name<U: Bound<T>>(&self) -> Option<&U> {
        self.as_ref().map(Bound::method)
    }
}

[nox]

I just meant that with this method, we have the solution opt.as_ref().borrowed(), it wasn't a mistake, will reformulate.

[nox]

I agree with @bluss that we likely want to leverage AsRef here instead of Borrow to allow for other conversions like Option<Cow<str>> to Option<&str> and such.

AFAIK, Cow implements Borrow, and AsRef doesn't have that many more implementations.

[nox]

It should also be noted that there is no implementation of AsRef<T> for T, which may or may not be a problem.

[lambda-fairy]

I'm not bothered by .as_refed() that much. After all, the convention of naming single-method traits as verbs (e.g. Read and Write) could be considered "ugly" as well, yet we got used to that over time.

If .as_refed() isn't feasible then these names could make good alternatives:

• .inner_as_ref() -- by analogy with .into_inner()
• .map_as_ref() -- contraction of .map(AsRef::as_ref)

[nox]

That would mean calling .as_ref().as_refed() which is even uglier.

I don't understand why AsRef is promoted as the way to go even for generic functions when that trait doesn't even have an impl for T.

[lambda-fairy]

On the other hand, the lack of a general identity impl lets AsRef cut through multiple layers of references in one go.

For example, you can convert from &&String to &str using AsRef but similar code using Borrow will fail. (playground)

Coherence stops us from adding a corresponding feature to Borrow. So for the use cases I'm familiar with -- various combinations of &, &mut, String, and str -- the AsRef solution is superior.

[nox]

How often do you hold onto Option<&&String> though?

[eddyb]

I really hope this ends up going through the coercion mechanism and not more ad-hoc hierarchies.

[oli-obk]

I really hope this ends up going through the coercion mechanism and not more ad-hoc hierarchies.

This please! Allow any enum Foo<T> { X(T), .. } value Foo<T> to coerce to Foo<U> if T can coerce to U (as long as Foo has no uses of T in subtypes where coercion isn't possible (like GenericStruct<T>)). All enum variants are public, so we could write a match by hand that does the coercion for every variant, so there's no reason to prevent it being done automatically.

[eddyb]

I would prefer it opt-in (like CoerceUnsized just generalized), but yes.

[nox]

Using Borrow means we could also have borrowed_mut (on Option<&mut T>) using BorrowMut, whereas as_muted using AsMut would also be quite weird.

As for implicit coercions, that's a rabbit hole I'm very wary of.

[nox]

It's not like we must limit ourselves to only one choice... Would people find Option::<&T>::dereferenced controversial?

[nox]

Any news on this?

[nox]

Happy new year. Was a decision reached for that?

[alilleybrinker]

nit: "along with other" rather than "along other."

[nox]

Changing the method to &self instead of self

What? Why?

[nox]

https://doc.rust-lang.org/std/option/enum.Option.html#method.cloned takes self, so this makes no sense to me. I'll do the other changes though.

[alexcrichton]

I commented above that I commented farther above with rationale for &self, not self

[alexcrichton]

er didn't mean to close

[Xion]

This RFC doesn't really make the common use case of "borrowing" Option<String> as Option<&str> (i.e. the stated goal in the motivation section) any more convenient.

To the best of my knowledge, the shortest way to do it currently is:

opt_string.as_ref().map(|s| s.as_str())

which this RFC improves only marginally to:

opt_string.as_ref().borrowed()

What I'd like to do is get rid of as_ref entirely here, and allow borrowed to coerce to whatever exact borrowed type is necessary in the current context. I'm not sure this is expressible in current Rust; my attempt would be somewhere along these lines:

impl<T> Option<T> {
    fn borrowed<U>(&self) -> Option<&U>
        where T: Borrow<U>
    {
        self.as_ref().map(|x| x.borrow())
    }
}

which may or may not be rephrasable to an equally powerful AsRef variant. (Since I don't think I understand the AsRef/Borrow distinction well enough, I'm preferring one over the other here).

[nox]

Changing the method to &self instead of self

If we do that, what happens for the type Option<&String>? We wouldn't be able to call borrowed anymore. I still don't think that is a good idea.

[daboross]

@nox If I understand correctly, this is a reason to do both &self and AsRef at once: AsRef works for Option<&String> to Option<&str> as well, and having the method accept &self would let it work for Option<String> too.

x.as_ref().map(|x| x.to_str()) really isn't that bad: is it worth adding an additional method if x.as_ref().borrowed() is still required?

[nox]

@daboross AsRef<T> does not allow getting a Option<&T> for all self: Option<&T>, as mentioned earlier. x.as_ref().map(|x| x.to_str()) really is bad, and my use cases aren't about strings anyway.

[daboross]

@nox Sorry, I know I should probably be getting this, but I don't seem to be.

Here's an reference implementation of borrowed with AsRef, which as far as I can tell works for all of the case you mentioned?

Option<&String>, Option<String> and Option<&str> can all be turned into Option<&str> with:

    fn borrowed<U: ?Sized>(&self) -> Option<&U>
        where T: AsRef<U>

http://play.integer32.com/?gist=6d1861f23259da72e8791a232e36ccc9

If there was a case I missed, could you elaborate on it specifically?

I really do want to find a good solution for this, I just can't find a place where AsRef doesn't work and Borrow does. As far as I can tell, AsRef just allows for more use cases.

[nox]

It doesn't work for arbitrary types.

…

-- Anthony Ramine

Le 22 mai 2017 à 09:10, David Ross ***@***.***> a écrit : If there was a case I missed, could you elaborate on it specifically?

[daboross]

OK - I think I understand now. The identity not being there with AsRef does make it more limiting (even though it opens up other options to).

If it isn't about the String use case though, do you think the RFC could be updated to remove that from the motivation?

I can get behind a Borrow-based method, but I don't think that it is the best solution if the only motivation in the RFC is for Option<&String> to Option<&str>. AsRef works just as well, and better, for the String, PathBuf and Vec use cases, and falls behind in the more general ones.

Thank you for clarifying that, and sorry I didn't completely get that before.

[aturon]

I'm sorry for the lack of follow-up here; it's been difficult to prioritize this particular API, given some of the issues we've covered on this thread. At this point, I'm inclined to postpone consideration; there's enough else going on that I'd like to let the dust settle a bit and see whether language-based solutions are shaping up.

@rfcbot fcp postpone

[rfcbot]

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

• @BurntSushi
• @Kimundi
• @alexcrichton
• @aturon
• @dtolnay
• @sfackler

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.

[nox]

I commented above that I commented farther above with rationale for &self, not self

@alexcrichton Sorry but I don't see a rationale in any of these two comments.

[rfcbot]

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

[rfcbot]

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

[rfcbot]

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

[rfcbot]

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

[aturon]

whoa there rfcbot...

[aturon]

I'm going to close this RFC as postponed. This is still a significant paper cut that would be great to fix, but work during the impl period on the trait system may provide an alternative, more general way of doing so. Thanks, @nox!