Make the crate no_std + alloc (rebased)
#48
Conversation
jonhoo
force-pushed
the
rebased-44
branch
2 times, most recently
from
432dcfb to
a3b1322
Compare
Codecov Report
|
|
Pushed some changes now that I think makes this patch a bit more pleasant. There is still the |
Can't we also provide a default to the |
|
Yeah, that's the problem -- on |
|
I guess the type alias helps hide the complexity, at the loss of available iterator types. I'm not sure if that's a net win -- I guess that's still driven by how desirable Did you manage to gather compelling use-cases? And can you prototype that this crate actually works in those cases? What |
|
I agree, it's definitely still a cost. As far as use-cases goes, the response was pretty meager. https://twitter.com/thequux/status/1223248563523723264 and https://twitter.com/thequux/status/1223248563523723264 were the primary ones. My current take is that we should probably just close this, with a note that we'd be open to considering if someone has a compelling use-case they're willing to also commit some time to supporting. |
I think that makes sense. |
This is necessary since there is no particular lock implementation that "makes sense" on `no_std`. This patch changes all types to be generic over any lock type that implements `lock_api::RawMutex`, and defaults to `parking_lot::RawMutex` on `std`. Note that this patch forks the struct definition for `HashMap` into one for `std` (which has a default for `L`) and one for `no_std` (which does not). This is fine since any code written _without_ the `std` feature _will_ compile with the `std` feature enabled (`indexmap` does the same; the reverse is not true). We _could_ introduce an intermediate private struct to avoid repeating the definition, but it would require writing `self.inner` all over the place, which would be sad. This seemed marginally cleaner. Also, this diff makes me want implied bounds a lot: rust-lang/rust#44491 This is essentially a rebase of adb58e0 and f9ad723 on top of the previous commit + #47.
On `no_std`, users are forced to use `HashMap::guard` anyway, since `epoch::pin` isn't available, so this seems fine.
This helps hide the `L` parameter even more. Unfortunately, I had to remove `IntoIterator` for `&ConcurrentHashMapRef` since `impl Trait` isn't supported in `type` position on trait impls yet.
|
All right, I'll close this and the corresponding issue then :) |
This is effectively a squash + rebase of #44 on top of #47.
It squashes the commits so that it is easier to see what's going on. @cuviper's comments from #44 (review) still apply, as does the question about whether we are even willing to make this change (#44 (comment)). There's a little twitter straw poll here. To summarize:
Thanks to @GarkGarcia for all the hard work on #44 -- that's the majority of this change.
Closes #44. Fixes #26.