derive_more
derive_more copied to clipboard
Add `Borrow` and `BorrowMut` derives
Had to switch from using AsRef to using Borrow in a project of mine for certain reasons and I'd really like to continue using this crate for that purpose. Borrow is identical in functionality to AsRef, but is used in different contexts and implemented for both T and &T.
This literally duplicates the functionality for AsRef, except the duplicated instances are replaced with Borrow. Same with AsMut and BorrowMut, which can be used like so:
#[derive(Borrow)]
struct MyStruct {
#[borrow]
data: u32
}
Let me know if there are any issues with this and I'll fix them. I think I got everything in the documentation included that is necessary, but I'm not 100% sure.
This could also be seen as a solution to #123 as AsRef<T> is not really suppose to be implemented for T or &T, but Borrow<T> is already blanket-implemented for T and &T (again, different contexts despite same behavior).
Any particular issues with this? Anything that would need to be changed to allow it to be merged in?
In particular Eq, Ord and Hash must be equivalent for borrowed and owned values:
x.borrow() == y.borrow()should give the same result asx == y.Borrow also requires that Hash, Eq and Ord for borrowed value are equivalent to those of the owned value. For this reason, if you want to borrow only a single field of a struct you can implement AsRef, but not Borrow.
This makes it seem like maybe
Borrowshould only be allowed to be derived on single field structs (i.e. newtypes). What do you think of that? Does it still cover your usecase then?
My usecase has actually somewhat changed and I am taking a different approach by using Deref instead. I can definitely say I was not ensuring Hash, Eq, or Ord were implemented properly though. I'm not sure if only allowing it for newtypes is the correct solution as the user still needs to ensure the other traits are implemented themselves.
Other than that the PR is missing some tests. You can probably copy most of the
as_ref/as_muttests: https://github.com/JelteF/derive_more/blob/master/tests/as_ref.rsThere's also the generics test, that you should probably add it to: https://github.com/JelteF/derive_more/blob/master/tests/generics.rs
I'm pretty busy at the moment, but I'll try to make some time for a full review of this PR.
I've added tests for Borrow and BorrowMut. I have not added anything to tests/generics.rs though as I do not see tests for AsRef or AsMut in that either.
I'm not sure if there's anyone else interested in this, but I've resolved the merge conflicts just in case.
I can say I'd appreciate a derive(Borrow), precisely in the single-field-struct case where it makes sense that a reference to the newtype "is" just a reference to the inner data. That said, a newtype may have additional fields that are ultimately secondary to the single "main" field, and don't compromise Hash/Eq/Ord semantics. So, I thought a bit about how to approach this situation.
Regarding the semantics of Borrow vs AsRef, I understand the concepts as such:
impl Borrow<Borrowed> for Type- Represents a reference to
Typeas a whole - Operations on
Borrowedare semantically equivalent to those onType - Impl is "canonical", i.e. there is at most* one impl for
Typebesides the blanket impl- *unless you make a very compelling argument otherwise, maybe
- Represents a reference to
impl AsRef<T> for Type- Represents a "view" of all or part of
Type Tmay be semantically distinct fromType; examples:Tis a more generic view ofType's underlying data (slices)Tis a distinct (possibly specialized) concept related toType(OsStr,Path, ...)Tis the same asBorrowed(usability with generic code)
- Variety of impls for any
Tthat can trivially representType's data- Generic code takes advantage of this, making interop easier overall
- Represents a "view" of all or part of
I established these principles from viewing the implementors of Borrow and AsRef in std, which I found quite helpful for more thoroughly understanding the traits' purposes. (Also, it's worth noting that Borrow's documentation states "A type is free to borrow as several different types." It's not really borne out by std's usage, but it's certainly possible to do idiomatically... and such idiomatic usage seems very unlikely to come up in practice. How many crates actually impl Borrow in the first place?)
I think a reasonable compromise, to better adhere to the spirit of Borrow, would be to limit the derived impl to precisely one field of the struct, rather than allowing one impl per unique Borrowed type, as is the case for derive(AsRef). This would allow "newtypes" with associated data to derive(Borrow), while limiting the possible damage of careless use of the macro.
With all the above said, restricting the derive to single-field structs is definitely the safest option; that's all I need personally (I don't have any examples of multi-field newtypes using Borrow), and some more boilerplate for more complex types is a reasonable cost. I don't think derive(Borrow) should be as lax as derive(AsRef), because of the risk of unsound semantics (both in terms of Eq etc. and because it doesn't make sense for a struct to borrow as multiple types which reference disjoint data within the struct). The comparative rarity of impl Borrow makes this a much more compelling tradeoff, in my opinion.
going over old PRs
With all the above said, restricting the derive to single-field structs is definitely the safest option; that's all I need personally (I don't have any examples of multi-field newtypes using
Borrow), and some more boilerplate for more complex types is a reasonable cost. I don't thinkderive(Borrow)should be as lax asderive(AsRef), because of the risk of unsound semantics (both in terms ofEqetc. and because it doesn't make sense for a struct to borrow as multiple types which reference disjoint data within the struct). The comparative rarity ofimpl Borrowmakes this a much more compelling tradeoff, in my opinion.
I totally agree with this. I think if someone changes this PR to add the single-field restriction then it can probably be merged quickly.