merge FixedDeref and StableAddress

[Storyyeller]

This crate is very similar to the owning_ref crate. In particular, they both define an unsafe trait to represent types which point to a fixed address (FixedDeref for rental and StableAddress for owning_ref). I think it would make sense to place this trait in a separate crate that you both depend on. This reduces code duplication and makes it easier for library authors to interact with these crates. For example, if I define a custom Rc type, I'd need to implement one trait to make it interoperable with rental and another for owning_ref. Putting StableAddress in a seperate crate will make things much nicer.

For reference, here are the impls of FixedDeref and StableAddress. The only difference is that StableAddress isn't impled for references, which is probably an oversight.

unsafe impl<'t, T: ?Sized> FixedDeref for &'t T { }
unsafe impl<'t, T: ?Sized> FixedDeref for &'t mut T { }

unsafe impl<T: ?Sized> FixedDeref for Box<T> { }
unsafe impl<T> FixedDeref for Vec<T> { }
unsafe impl FixedDeref for String { }

unsafe impl<T: ?Sized> FixedDeref for rc::Rc<T> { }
unsafe impl<T: ?Sized> FixedDeref for sync::Arc<T> { }

unsafe impl<'t, T: ?Sized> FixedDeref for cell::Ref<'t, T> { }
unsafe impl<'t, T: ?Sized> FixedDeref for cell::RefMut<'t, T> { }
unsafe impl<'t, T: ?Sized> FixedDeref for sync::MutexGuard<'t, T> { }
unsafe impl<'t, T: ?Sized> FixedDeref for sync::RwLockReadGuard<'t, T> { }
unsafe impl<'t, T: ?Sized> FixedDeref for sync::RwLockWriteGuard<'t, T> { }

unsafe impl<T: ?Sized> StableAddress for Box<T> {}
unsafe impl<T> StableAddress for Vec<T> {}
unsafe impl StableAddress for String {}

unsafe impl<T: ?Sized> StableAddress for Rc<T> {}
unsafe impl<T: ?Sized> CloneStableAddress for Rc<T> {}
unsafe impl<T: ?Sized> StableAddress for Arc<T> {}
unsafe impl<T: ?Sized> CloneStableAddress for Arc<T> {}

unsafe impl<'a, T: ?Sized> StableAddress for Ref<'a, T> {}
unsafe impl<'a, T: ?Sized> StableAddress for RefMut<'a, T> {}
unsafe impl<'a, T: ?Sized> StableAddress for MutexGuard<'a, T> {}
unsafe impl<'a, T: ?Sized> StableAddress for RwLockReadGuard<'a, T> {}
unsafe impl<'a, T: ?Sized> StableAddress for RwLockWriteGuard<'a, T> {}

[jpernst]

Sorry for missing this, for some reason notifications on this repo were disabled.

This would require coordination and buy-in from the author of owning_ref, but I'm not opposed to it. Our crates probably shouldn't be dependencies of eachother, so it would require splitting off a separate crate just for the trait.

[Storyyeller]

Our crates probably shouldn't be dependencies of eachother, so it would require splitting off a separate crate just for the trait.

That's my plan, assuming I can get buy-in from the owner of owning_ref. I'd be happy to create the crate and PR and give you ownership if necessary.

[Storyyeller]

Here's the draft version of the new crate: https://github.com/Storyyeller/stable_address_trait/blob/master/src/lib.rs

I wrote a bunch of documentation about the requirements for the trait. I'd like for you to make sure it is correct and aligns with the assumptions made by rental.

If you think it's ok, I'll go ahead and publish it on crates.io and then send a PR to update rental to use it.

[jpernst]

Implementation looks good to me, but to bikeshed the name a bit, I think StableDeref might be better, since it emphasizes the fact that the Deref target must remain identical, not necessarily the struct itself.

EDIT: Or perhaps DerefStable instead, since things like DerefMut put the Deref part first. Either way really.

[jpernst]

Oh, one other thing I forgot, you'll probably want to add no_std support, since I've committed to that for rental going forward.

[Storyyeller]

I went ahead and renamed to StableDeref and added no_std support.

Though I do wonder how useful it will be on no_std, given that every impl except for &T and &mut T requires std.

[jpernst]

Yeah, in a no_std environment they'd presumably be using their own specialized allocators and would need to manually add trait impls to support them.

In any case, I'd say it looks good now.

[Storyyeller]

FYI, I looked through the dependent crates on crates.io to find any that are using the trait. It turns out there are two: parking_lot and shawshank. parking_lot has an optional dependency on owning_ref to impl StableAddress for the custom mutexes it defines (so it should benefit a lot from this refactoring). shawshank uses StableAddress as the bound for its own ArenaSet type.

I'm planning to add a restriction to the StableDeref documentation that if deref() is called multiple times, it must return the same address each time. owning_ref and rental don't depend on this property as far as I can tell, but shawshank does.

Anyway, it's probably not important to you, but I figured I should fill you in.

[jpernst]

Sounds good; that's behavior I'd expect from this trait anyway, and would be quite surprised if it weren't the case.