implement Entry API for SmallIntMap

[Gankra]

The contents of this PR make me legit mad. I really hope that I'm just completely stupid tonight, and there's a way better way to do this. It doesn't help that this API is basically pointless to provide on SmallIntMap. But we've gotta provide a uniform interface, so...

yeah

😡

[rust-highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[bkoropoff]

Have you tried something like:

{
    match self.find_mut(&key) {
        Some(val) => return Occupied(...)
        None => {}
    }
}
Vacant(...)

EDIT: It doesn't work. I'm surprised the borrow "bleeds" out of the enclosing block.
EDIT EDIT: I think it's because the region of the returned expression becomes linked to that of the return value of the function, which is the same as that of self ('a), which is linked to the region of the function body. This ends up extending the lifetime. Come to think of it, I think I've seen people complain about this before.

[bkoropoff]

Your borrow checker pain seems to be issue #6393. The problem attempting to use return as a workaround is called out as a particularly tricky case.

[gereeter]

Another alternative would be storing &'a mut Option<V>, which would be totally safe but would require unwrapping the Option.

[Gankra]

Still need the ref to decrement len :(

[gereeter]

What len is being decremented? SmallIntMap doesn't cache the number of set elements.

[Gankra]

@gereeter Oh! You know, I just totally assumed. That makes the Option solution much more viable, though the unwraps are unsavoury.

[pcwalton]

r? @aturon

[aturon]

I spent a few minutes trying to find a better way around the borrow checker here, but to no avail. This code also makes me sad, but at least it's all hidden behind the API. Someday we'll be able to do this better...

[Gankra]

Sorry about that. Fixed, squashed, and make checked.

[Gankra]

I don't understand the error that was spat out:

error: the trait `core::iter::FromIterator<(int,int)>` is not implemented for the type `smallintmap::SmallIntMap<int>`

let mut map: SmallIntMap<int> = xs.iter().map(|&x| x).collect();

I didn't rebase to latest master, could something odd have changed with resolution?

[aturon]

@gankro Method resolution has been in flux, so possibly? Definitely worth rebasing and seeing if you can reproduce.

[Gankra]

Rebased to master, still works. I'm at a loss.

[reem]

Is SmallIntMap really even worth having? It's basically just Vec.

[Gankra]

@reem I've been pushing for stuff to be pulled when we start stabilizing. SmallIntMap I could take or leave. It's pretty easy for a client to write safely, but also has low maintenance cost.

Until then, I'm forced to assume it will stay.

[reem]

What's the point? It offers almost no added utility over Vec<Option<T>>

[Gankra]

@reem It's easier to use than a Vec<Option<T>>, and is faster to use than a heavier data structure. iirc rustc uses it. It's been suggested that some of these "rustc wants it" structures could be left in as #[experimental] so that we're making no promises to maintain/keep it outside. I dunno. Something we need to work out.

[aturon]

Even if we decide not to stabilize it, we'd move it out to an external Cargo crate, so I think it's still worth making the API consistent with the other collections.

[aturon]

@gankro I will try to take a look at this failure ASAP.

[Gankra]

Killing this while collections reform is on.