jeromefroe / lru-rs

An implementation of a LRU cache
https://docs.rs/lru/
MIT License
648 stars 106 forks source link

Remove KeyRef from the public api #166

Closed jonhartnett closed 1 year ago

jonhartnett commented 1 year ago

As best I can tell, KeyRef is part of the public api (albeit hidden) because we can't create a "blanket" Borrow impl for KeyRef because it conflicts with the stdlib blanket impl:

//doesn't compile
impl<K, Q> Borrow<Q> for KeyRef<K>
where
    K: Borrow<Q>,
    Q: ?Sized,
{
    // ...
}

However, we can circumvent this issue by introducing a transparent wrapper impl around the other side:

//does compile
impl<K, Q> Borrow<KeyWrapper<Q>> for KeyRef<K>
where
    K: Borrow<Q>,
    Q: ?Sized,
{
    // ...
}

This allows us to eliminate the KeyRef<K>: Borrow<Q> constraints in favor of the more flexible K: Borrow<Q>. It also removes the need to have KeyRef be part of the public api, hidden or otherwise.

Please let me know if you have any comments on the idea or the impl.

jeromefroe commented 1 year ago

Thanks for the contribution @jonhartnett! These changes look good to me, mind just fixing the formatting that caused the CI check to fail? After that I think we should be all set to merge.

jonhartnett commented 1 year ago

Done

jeromefroe commented 1 year ago

Just created a new release, 0.10.0, with these changes.