Make overlapping/overlaps accept ranges by-value instead of by-reference

[clarfonthey]

Breaking change, but I find this kind of change necessary to interface properly with this API.

Main reasons:

1. These ranges will primarily be of integers, who are Copy anyway.
2. Without taking ownership of the range, generating one on-the-fly is basically impossible.

Essentially, if you have an iterator method which uses overlapping internally, it will only work properly if you pass in a reference to a range directly from the top level; anything else will be insufficient, unless you want to use Box::leak, which is quite wasteful.

[jeffparsons]

On mobile, so this will be brief.

I'm afraid I don't understand from your description the limitation you've run into. Could you please provide example code for what you'd attempt with the current interface that doesn't work? (Super rough is fine; I'm just trying to understand which bit doesn't fit.)

[clarfonthey]

Sure, here's something rather simple from what I was doing:

struct Addrs {
    ipv4: RangeInclusiveSet<u32>,
    // ...
}
fn range_v4(net: Ipv4Net) -> RangeInclusive<u32> {
    // ...
}
fn map_v4(r: &RangeInclusive<u32>) -> RangeInclusive<Ipv4Addr> {
    // ...
}
fn overlapping_v4(&self, net: Ipv4Net) -> impl '_ + Iterator<Item = RangeInclusive<Ipv4Addr>> {
    self.ipv4.overlapping(range_v4(net)).map(map_v4)
}

Here, I'm using u32 inside a RangeInclusiveSet to represent Ipv4Addr ranges, and writing a method that returns an iterator over Ipv4Addr ranges. (Note: this problem still exists even if Ipv4Addr implemented Step directly, but I'm using u32 internally to get past that bit.)

Since the range is being created from another type (in this case, Ipv4Net) instead of being passed into the parent method directly, I can't just borrow the range, since it wouldn't outlive this method.

Basically, any iterator adapter that would want to create its own range instead of having one passed from the caller would have this problem. Since you're creating the range inside the method, you can't actually create an Overlapping iterator, since it needs something to borrow for the entire life of the iterator.

[ripytide]

Could you store the range you generated in the method inside a wrapping struct which you could then impl Iterator on to ensure the range used would always live as long as the iterator adapter?

That aside, I deliberated on using references or owned values quite a bit in DiscreteRangeMap but since I can only imagine people using Copy values as you said I went with owned values for slightly nicer ergonomics.

[clarfonthey]

Could you store the range you generated in the method inside a wrapping struct which you could then impl Iterator on to ensure the range used would always live as long as the iterator adapter?

So this would be a possible solution except that it would mean that instead of returning something along the lines of type MyIter = impl Iterator, it would be type MyIter = impl where for<'a> &'a Self: IntoIterator.

Which… excusing the fact that this isn't really possible in today's Rust, is a really awkward way to justify not wanting to clone something.

Elaborating a bit more on how this would work, your method would return a struct which contains the generated range, and then references to that struct could be converted into a different struct which is iterable, and that conversion would be what calls the overlapping method using the reference to the range.

[clarfonthey]

I'm going to close this since the master branch has been force-pushed since this change, and because it's been left in limbo for over a year with other changes being merged similar to it in the meantime. Feel free to implement it yourself.

[jeffparsons]

@clarfonthey Sorry about leaving this in limbo; I dropped the ball here.

Speaking more broadly, I have very little free time these days so I've been taking a super-conservative approach to maintenance: basically just bug fixes for now. I should probably declare this in the readme.

Further down the line I expect life circumstances to allow a bit more time. Then I'll certainly take this into account, and other breaking changes that might be worthwhile to take advantage of cursors, learnings from other crates, etc.

[clarfonthey]

Oh, life circumstances are extremely understandable; I've got a lot of those myself.

I mostly just have been closing old PRs so I can sort of keep track in my open PRs tab.