Borrow and AsRef are redundant

[nwin]

These traits have exactly the same signature, both are stable and both should be implemented by the same types. One of them should go.

Furthermore AsRef is not implemented by Box etc…

[aturon]

There are a few really important differences:

• Borrow is used by data structures like HashMap and BTreeMap which assume that the result of hashing or ordering based on owned and borrowed values is the same. But there are many AsRef conversions for which this isn't true. So in general, Borrow should be implemented for fewer types than AsRef, because it makes stronger guarantees.
• Borrow provides a blanket implementation T: Borrow<T>, which is essential for making the above collections work well. AsRef provides a different blanket implementation, basically &T: AsRef<U> whenever T: AsRef<U>, which is important for APIs like fs::open that can use a simpler and more flexible signature as a result. You can't have both blanket implementations due to coherence, so each trait is making the choice that's appropriate for its use case.

This should be better documented, but there's definitely a reason that things are set up this way.

As to AsRef on Box and friends, the initial implementations were somewhat minimal, and we've been adding more over time.

I'm going to close this issue for now, since the traits are not in fact redundant. But it may be worth opening a separate issue for improved documentation and missing impls on AsRef.

[frewsxcv]

For the sake of cross-posting, I just asked this same question in Discourse without knowing this Issue existed.

[quadrupleslap]

Still not properly documented (I think.)

Edit: The only explanation I can find is on the AsRef page and it's much more vague than @aturon's amazing explanation here.

[steveklabnik]

https://doc.rust-lang.org/book/first-edition/borrow-and-asref.html exists, but it's not going to in the second edition

[prasannavl]

@aturon - Thanks for a better explanation - However from what I understand the only place where Borrow really differs is while borrowing a reference for hashing. So would it not make sense to have the trait be AsHashableRef (or something similar) and method be as_hashable_ref or something similar? Hopefully some one else can come up with a shorter name that makes sense.

It is rather verbose - but it's intentions are much more clear. Borrow is just confusing in the context.

PS: I could be wrong, but my brain is likely misleading me into thinking I have good understanding after reading the above statement, but I'm not entirely sure if I'm clear on the kind of consequences on implementing one over the other would cause. Would be better if there are side-by-side comparisons of both, in the documentation to express it clearly.

[prasannavl]

Ah. Alright, I realize I was wrong - and my above statement is incorrect.

Finally, this was one only post that provided the most sensible documentation so far:
#44868

Thanks to @stjepang for this.

PS: All other documentation including the above comment just focuses so much only hasing - it still doesn't provide a complete picture and possibly misleading - even though it's right.

[partim]

I’m not sure if this is the best place to discuss this, but as part of working on #44868, I’d like to verify the precise guarantees made by Borrow. My feeling is that the set of traits for which, if they are implemented, both types must exhibit identical behavior, is larger than just the comparison traits.

In other words: Has Borrow been devised exclusively for collections or is there wider use cases?

[partim]

Thinking about this some more: The most strict guarantee would probably be: ‘If T: Borrow<U>, then for every trait that is implemented by both T and U, every trait method that takes a &self argument must exhibit the exact same behavior for both T and U.’ A similar guarantee should then be stated for BorrowMut with &mut self.

While not stated currently in the documentation, this is perhaps what is intended?

[mikeyhew]

I'm a bit troubled by @aturon's comment about Borrow and Hash and Ord:

Borrow is used by data structures like HashMap and BTreeMap which assume that the result of hashing or ordering based on owned and borrowed values is the same.

I was surprised to hear that types implementing Borrow<T> have to hash and be ordered the same way as T. I see now that it is mentioned in the documentation for Borrow, but it seems like a restriction that is unrelated to the concept of borrowing data, and if HashMap and BTreeMap require those invariants, they should use separate marker traits for them.

I realize it's not possible to add another trait bound to HashMap and BTreeMap at this point, and even if it was, it might not be desirable. It's just that this hurts the Borrow trait by making it less general, and seems prone to error by people like me who implement Borrow for their type without noticing that restriction in the documentation. The latter problem could be addressed by making the following sentence stand out more in the docs:

If you are implementing Borrow and both Self and Borrowed implement Hash, Eq, and/or Ord, they must produce the same result.

[partim]

@mikeyhew If you don’t want to make the trait-related guarantees, you probably should implement AsRef. Borrow really should only ever be implemented by smart pointers or other types that manage data without changing its behavior at all. I agree that the naming is a bit unfortunate (though, AsRef is at least a tiny bit shorter), but we are stuck with that.

I tried to make the distinction more clear in the proposed new text in #46518.

[mikeyhew]

@partim AsRef<T> isn't implemented for T though, which doesn't matter when it's used for a function argument that you are going to immediately convert to a reference, but does when you are storing it in some sort of data structure, which isn't necessarily a HashMap or BTreeMap. I mean, there might not be many types that would implement Borrow but hash differently: the only case that I can think of is if you have a struct with two fields that implements Borrow for each, and I'm not sure how useful that is in practice.

[leoyvens]

Triage: Now that #46518 has landed, can this be closed or at least de-prioritized?

[steveklabnik]

Yes, closing in favor of that. If there are still improvements to be made, please open new issues!