Fix two unsoundness bugs

[jonhoo]

Fixes both #46 and the issue highlighted here (K and V must both be 'static in case of a global collector!).

@cuviper I wonder if we could do something cool with this change now that we have the Collector. I'm specifically thinking something like HashMap::pin, which returns a HashMapRef (like in #45), which internally holds a LocalHandle and a Guard (not sure how to make the lifetimes work out).

[codecov]

Codecov Report

Merging #49 into master will increase coverage by 0.95%.
The diff coverage is 88.57%.

Impacted Files	Coverage Δ
src/map.rs	86.2% <88.57%> (+1.31%)	⬆️

[cuviper]

Ugh, that's awkward that Collectors don't have an actual lifetime in them. I'm trying to think of other ways we can deal with this, but I'm not sure.

I'm specifically thinking something like HashMap::pin, which returns a HashMapRef (like in #45), which internally holds a LocalHandle and a Guard (not sure how to make the lifetimes work out).

There are no lifetimes between those either, so it should work just fine to hold both in the same struct. I guess you don't even really need the LocalHandle at that point, just the Guard, unless you want to expose that handle to the user.

[cuviper]

As a first, minimal step, I'd suggest just requiring 'static types and assert that Guards are from the global collector. Custom collectors complicate the correct usage of the map, as now you have to go through the long register path to get a guard. (Unless you know it still has the default collector, but this isn't represented at all at the type level.)

[jonhoo]

I think this change is strictly more general than it was previously? If you just use HashMap::new and friends (basically, if you never call with_constructor), then you can always just use epoch::pin like you did before (and not go through register). Or am I missing something?

It might be nice to provide HashMap::guard which just directly gives you a Guard too.

[cuviper]

If you just use HashMap::new and friends (basically, if you never call with_constructor), then you can always just use epoch::pin like you did before (and not go through register). Or am I missing something?

You can only assume that if you know that the map was never in the hands of anything that might have called with_constructor. That means both controlling self and &mut self (because of swap) at all times. Maybe that's paranoid of me, but I don't like that this is a transparent footgun.

[jonhoo]

Yes, I think any code that takes a HashMap must use register. You're not wrong that it's a potential foot-gun! How about we remove with_collector for the time being, given that (I think) that is the only thing currently blocking this? We still leave the Collector in there for the purposes of HashMapRef.

[cuviper]

Looks good to me!