rangemap icon indicating copy to clipboard operation
rangemap copied to clipboard

Conform to borrowing convention from `std`

Open jeffparsons opened this issue 6 years ago • 3 comments

BTreeMap has signatures like...

    pub fn get<Q: ?Sized>(&self, key: &Q) -> Option<&V>
        where K: Borrow<Q>,
              Q: Ord

Using Borrow in RangeMap and RangeSet would be better than the plain borrows it has now.

jeffparsons avatar Feb 15 '19 10:02 jeffparsons

I think this might not actually be possible, because core::ops::Range::contains doesn't use Borrow.

Compare:

  • https://doc.rust-lang.org/stable/std/collections/struct.BTreeSet.html#method.contains
  • https://doc.rust-lang.org/stable/std/ops/struct.Range.html#method.contains

And we would need to be able to pass a Q into core::ops::Range::contains.

I'm closing this for now. If someone else points out that it's actually possible somehow then I'll re-open it. But it should be backward compatible, so I'm no longer treating it as a 1.0-blocker just for completeness reasons.

jeffparsons avatar Aug 23 '21 12:08 jeffparsons

On second thoughts, this limitation only exists because of my current internal representation (and even with it I think I could work around it), so I'm going to re-open this. I'm thinking of trying a different internal representation anyway, so we'll see if that goes anywhere.

jeffparsons avatar Aug 24 '21 10:08 jeffparsons

My attempts at implementing Borrow in ways that would let me work around this keep ending up conflicting with this blanket impl:

impl<T: ?Sized> Borrow<T> for T { ... }

I could hack around it on nightly using negative impls or specialization, but I haven't found any way to get around it on stable. I think I'll just let this sit for now and come back to it when one of those is stabilized, unless someone else comes up with a clever workaround in the meantime. :)

jeffparsons avatar Aug 30 '21 08:08 jeffparsons