Steal good commits from #44 #47
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
January 30, 2020 15:38
Now the `S` type parameter is only required to implement `Default`.
This makes the methods more consistent with one another, allows for greater efficiency, and are necessary for potential eventual `no_std` support.
We choose this over `std`'s `RandomState` for two reasons:
1. It is faster, and the DoS protection will be provided by the (as of
yet unimplemented) `TreeBin` optimization.
2. It is more amenable to `no_std` environments.
Codecov Report
|
jonhoo
added a commit
that referenced
this pull request
Jan 30, 2020
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.
This was referenced Jan 30, 2020
jonhoo
added a commit
that referenced
this pull request
Jan 30, 2020
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.
jonhoo
added a commit
that referenced
this pull request
Jan 30, 2020
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.
|
Awesome! I'm happy I could help 😃 |
jonhoo
added a commit
that referenced
this pull request
Feb 1, 2020
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.
jonhoo
added a commit
that referenced
this pull request
Feb 3, 2020
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This steals a bunch of the commits from #44 that are not related to
no_std, and should land independently of whether we mergeno_stdsupport.