-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Relax some Hash bounds on HashMap<K, V, S> and HashSet<T, S> #58370
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
r? @Amanieu |
This comment has been minimized.
This comment has been minimized.
Could you give a full example of when this would be useful? In particular, to construct a |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Short answer: I shouldn't have to add Long answer: I do fancy things in this project of mine, in which I pointer cast Also, all other collections (apart from |
That is dealt with by #44491.
Ah; thanks. Btw.. |
I don't think you can call that "dealt with", given that means adding bounds to that collection when, say, |
Yeah, that's fair. |
src/libstd/collections/hash/map.rs
Outdated
/// let hasher: &RandomState = map.hasher(); | ||
/// ``` | ||
#[stable(feature = "hashmap_public_hasher", since = "1.9.0")] | ||
pub fn hasher(&self) -> &S { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if this function kept the S: BuildHasher
bound. I have some changes to hashbrown (which will soon replace the libstd hashmap) which may require this bound in the future.
Do you really need to call map.hasher()
without a S: BuildHasher
bound? If you really need it then I can try looking into a workaround, but it might get a bit ugly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need to call
map.hasher()
without aS: BuildHasher
bound? If you really need it then I can try looking into a workaround, but it might get a bit ugly.
Nope, it was just one of the methods I saw that didn't need a bound, I can add it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it back where it was before with the same bounds.
Apart from my concern on |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Assuming the change for @rfcbot fcp merge |
6879128
to
82fb122
Compare
Awesome! I just wanted this the other day 🙂 (Same kind of things as nox already mentioned: passed a |
Pushed a commit that I didn't intend to push directly in that PR, but I guess maybe I should? I did a similar patch for |
Sorry, I meant |
☔ The latest upstream changes (presumably #58341) made this pull request unmergeable. Please resolve the merge conflicts. |
Notably, hash iterators don't require any trait bounds to be iterated.
@rfcbot reviewed under the understanding that the set of API changes is: impl<K, V, S> HashMap<K, V, S>
- where
- K: Eq + Hash,
- S: BuildHasher,
{
fn capacity(&self) -> usize;
fn keys(&self) -> Keys<K, V>;
fn values(&self) -> Values<K, V>;
fn values_mut(&mut self) -> ValuesMut<K, V>;
fn iter(&self) -> Iter<K, V>;
fn iter_mut(&mut self) -> IterMut<K, V>;
fn len(&self) -> usize;
fn is_empty(&self) -> bool;
fn drain(&mut self) -> Drain<K, V>;
fn clear(&mut self);
}
impl<'a, K, V, S> IntoIterator for &'a HashMap<K, V, S>
- where
- K: Eq + Hash,
- S: BuildHasher;
impl<'a, K, V, S> IntoIterator for &'a mut HashMap<K, V, S>
- where
- K: Eq + Hash,
- S: BuildHasher;
impl<K, V, S> IntoIterator for HashMap<K, V, S>
- where
- K: Eq + Hash,
- S: BuildHasher;
impl<T, S> HashSet<T, S>
- where
- T: Eq + Hash,
- S: BuildHasher,
{
fn capacity(&self) -> usize;
fn iter(&self) -> Iter<T>;
fn len(&self) -> usize;
fn is_empty(&self) -> bool;
fn drain(&mut self) -> Drain<T>;
fn clear(&mut self);
}
impl<'a, T, S> IntoIterator for &'a HashSet<T, S>
- where
- T: Eq + Hash,
- S: BuildHasher;
impl<T, S> IntoIterator for HashSet<T, S>
- where
- T: Eq + Hash,
- S: BuildHasher; |
Edit: never mind, this isn't like Vec::drain where there can be items left afterward. |
Currently |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process,I would like to thank @sfacklerfor their work and everyone else who contributed. The RFC will be merged soon. |
@bors r+ |
📌 Commit d9e2259 has been approved by |
Relax some Hash bounds on HashMap<K, V, S> and HashSet<T, S> Notably, hash iterators don't require any trait bounds to be iterated.
Rollup of 5 pull requests Successful merges: - #58370 (Relax some Hash bounds on HashMap<K, V, S> and HashSet<T, S>) - #58421 (Relax some Ord bounds on BinaryHeap<T>) - #58686 (replace deprecated rustfmt_skip with rustfmt::skip) - #58697 (Use ? in some macros) - #58704 (Remove some unnecessary 'extern crate') Failed merges: r? @ghost
Pkgsrc changes: * Bump required rust version to build to 1.33.0. * Adapt patches to changed file locations. * (I worry about 32-bit ports, now that Atomic64 apparently is First-Class; this has been built on NetBSD/amd64 so far) Upstream changes: Version 1.34.0 (2019-04-11) ========================== Language -------- - [You can now use `#[deprecated = "reason"]`][58166] as a shorthand for `#[deprecated(note = "reason")]`. This was previously allowed by mistake but had no effect. - [You can now accept token streams in `#[attr()]`,`#[attr[]]`, and `#[attr{}]` procedural macros.][57367] - [You can now write `extern crate self as foo;`][57407] to import your crate's root into the extern prelude. Compiler -------- - [You can now target `riscv64imac-unknown-none-elf` and `riscv64gc-unknown-none-elf`.][58406] - [You can now enable linker plugin LTO optimisations with `-C linker-plugin-lto`.][58057] This allows rustc to compile your Rust code into LLVM bitcode allowing LLVM to perform LTO optimisations across C/C++ FFI boundaries. - [You can now target `powerpc64-unknown-freebsd`.][57809] Libraries --------- - [The trait bounds have been removed on some of `HashMap<K, V, S>`'s and `HashSet<T, S>`'s basic methods.][58370] Most notably you no longer require the `Hash` trait to create an iterator. - [The `Ord` trait bounds have been removed on some of `BinaryHeap<T>`'s basic methods.][58421] Most notably you no longer require the `Ord` trait to create an iterator. - [The methods `overflowing_neg` and `wrapping_neg` are now `const` functions for all numeric types.][58044] - [Indexing a `str` is now generic over all types that implement `SliceIndex<str>`.][57604] - [`str::trim`, `str::trim_matches`, `str::trim_{start, end}`, and `str::trim_{start, end}_matches` are now `#[must_use]`][57106] and will produce a warning if their returning type is unused. - [The methods `checked_pow`, `saturating_pow`, `wrapping_pow`, and `overflowing_pow` are now available for all numeric types.][57873] These are equivalvent to methods such as `wrapping_add` for the `pow` operation. Stabilized APIs --------------- #### std & core * [`Any::type_id`] * [`Error::type_id`] * [`atomic::AtomicI16`] * [`atomic::AtomicI32`] * [`atomic::AtomicI64`] * [`atomic::AtomicI8`] * [`atomic::AtomicU16`] * [`atomic::AtomicU32`] * [`atomic::AtomicU64`] * [`atomic::AtomicU8`] * [`convert::Infallible`] * [`convert::TryFrom`] * [`convert::TryInto`] * [`iter::from_fn`] * [`iter::successors`] * [`num::NonZeroI128`] * [`num::NonZeroI16`] * [`num::NonZeroI32`] * [`num::NonZeroI64`] * [`num::NonZeroI8`] * [`num::NonZeroIsize`] * [`slice::sort_by_cached_key`] * [`str::escape_debug`] * [`str::escape_default`] * [`str::escape_unicode`] * [`str::split_ascii_whitespace`] #### std * [`Instant::checked_add`] * [`Instant::checked_sub`] * [`SystemTime::checked_add`] * [`SystemTime::checked_sub`] Cargo ----- - [You can now use alternative registries to crates.io.][cargo/6654] Misc ---- - [You can now use the `?` operator in your documentation tests without manually adding `fn main() -> Result<(), _> {}`.][56470] Compatibility Notes ------------------- - [`Command::before_exec` is now deprecated in favor of the unsafe method `Command::pre_exec`.][58059] - [Use of `ATOMIC_{BOOL, ISIZE, USIZE}_INIT` is now deprecated.][57425] As you can now use `const` functions in `static` variables. [58370]: rust-lang/rust#58370 [58406]: rust-lang/rust#58406 [58421]: rust-lang/rust#58421 [58166]: rust-lang/rust#58166 [58044]: rust-lang/rust#58044 [58057]: rust-lang/rust#58057 [58059]: rust-lang/rust#58059 [57809]: rust-lang/rust#57809 [57873]: rust-lang/rust#57873 [57604]: rust-lang/rust#57604 [57367]: rust-lang/rust#57367 [57407]: rust-lang/rust#57407 [57425]: rust-lang/rust#57425 [57106]: rust-lang/rust#57106 [56470]: rust-lang/rust#56470 [cargo/6654]: rust-lang/cargo#6654 [`Any::type_id`]: https://doc.rust-lang.org/std/any/trait.Any.html#tymethod.type_id [`Error::type_id`]: https://doc.rust-lang.org/std/error/trait.Error.html#tymethod.type_id [`atomic::AtomicI16`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI16.html [`atomic::AtomicI32`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI32.html [`atomic::AtomicI64`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI64.html [`atomic::AtomicI8`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI8.html [`atomic::AtomicU16`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU16.html [`atomic::AtomicU32`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU32.html [`atomic::AtomicU64`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU64.html [`atomic::AtomicU8`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU8.html [`convert::Infallible`]: https://doc.rust-lang.org/std/convert/enum.Infallible.html [`convert::TryFrom`]: https://doc.rust-lang.org/std/convert/trait.TryFrom.html [`convert::TryInto`]: https://doc.rust-lang.org/std/convert/trait.TryInto.html [`iter::from_fn`]: https://doc.rust-lang.org/std/iter/fn.from_fn.html [`iter::successors`]: https://doc.rust-lang.org/std/iter/fn.successors.html [`num::NonZeroI128`]: https://doc.rust-lang.org/std/num/struct.NonZeroI128.html [`num::NonZeroI16`]: https://doc.rust-lang.org/std/num/struct.NonZeroI16.html [`num::NonZeroI32`]: https://doc.rust-lang.org/std/num/struct.NonZeroI32.html [`num::NonZeroI64`]: https://doc.rust-lang.org/std/num/struct.NonZeroI64.html [`num::NonZeroI8`]: https://doc.rust-lang.org/std/num/struct.NonZeroI8.html [`num::NonZeroIsize`]: https://doc.rust-lang.org/std/num/struct.NonZeroIsize.html [`slice::sort_by_cached_key`]: https://doc.rust-lang.org/std/slice/fn.sort_by_cached_key [`str::escape_debug`]: https://doc.rust-lang.org/std/primitive.str.html#method.escape_debug [`str::escape_default`]: https://doc.rust-lang.org/std/primitive.str.html#method.escape_default [`str::escape_unicode`]: https://doc.rust-lang.org/std/primitive.str.html#method.escape_unicode [`str::split_ascii_whitespace`]: https://doc.rust-lang.org/std/primitive.str.html#method.split_ascii_whitespace [`Instant::checked_add`]: https://doc.rust-lang.org/std/time/struct.Instant.html#method.checked_add [`Instant::checked_sub`]: https://doc.rust-lang.org/std/time/struct.Instant.html#method.checked_sub [`SystemTime::checked_add`]: https://doc.rust-lang.org/std/time/struct.SystemTime.html#method.checked_add [`SystemTime::checked_sub`]: https://doc.rust-lang.org/std/time/struct.SystemTime.html#method.checked_sub
Pkgsrc changes: * Bump required rust version to build to 1.33.0. * Adapt patches to changed file locations. * (I worry about 32-bit ports, now that Atomic64 apparently is First-Class; this has been built on NetBSD/amd64 so far) Upstream changes: Version 1.34.0 (2019-04-11) ========================== Language -------- - [You can now use `#[deprecated = "reason"]`][58166] as a shorthand for `#[deprecated(note = "reason")]`. This was previously allowed by mistake but had no effect. - [You can now accept token streams in `#[attr()]`,`#[attr[]]`, and `#[attr{}]` procedural macros.][57367] - [You can now write `extern crate self as foo;`][57407] to import your crate's root into the extern prelude. Compiler -------- - [You can now target `riscv64imac-unknown-none-elf` and `riscv64gc-unknown-none-elf`.][58406] - [You can now enable linker plugin LTO optimisations with `-C linker-plugin-lto`.][58057] This allows rustc to compile your Rust code into LLVM bitcode allowing LLVM to perform LTO optimisations across C/C++ FFI boundaries. - [You can now target `powerpc64-unknown-freebsd`.][57809] Libraries --------- - [The trait bounds have been removed on some of `HashMap<K, V, S>`'s and `HashSet<T, S>`'s basic methods.][58370] Most notably you no longer require the `Hash` trait to create an iterator. - [The `Ord` trait bounds have been removed on some of `BinaryHeap<T>`'s basic methods.][58421] Most notably you no longer require the `Ord` trait to create an iterator. - [The methods `overflowing_neg` and `wrapping_neg` are now `const` functions for all numeric types.][58044] - [Indexing a `str` is now generic over all types that implement `SliceIndex<str>`.][57604] - [`str::trim`, `str::trim_matches`, `str::trim_{start, end}`, and `str::trim_{start, end}_matches` are now `#[must_use]`][57106] and will produce a warning if their returning type is unused. - [The methods `checked_pow`, `saturating_pow`, `wrapping_pow`, and `overflowing_pow` are now available for all numeric types.][57873] These are equivalvent to methods such as `wrapping_add` for the `pow` operation. Stabilized APIs --------------- #### std & core * [`Any::type_id`] * [`Error::type_id`] * [`atomic::AtomicI16`] * [`atomic::AtomicI32`] * [`atomic::AtomicI64`] * [`atomic::AtomicI8`] * [`atomic::AtomicU16`] * [`atomic::AtomicU32`] * [`atomic::AtomicU64`] * [`atomic::AtomicU8`] * [`convert::Infallible`] * [`convert::TryFrom`] * [`convert::TryInto`] * [`iter::from_fn`] * [`iter::successors`] * [`num::NonZeroI128`] * [`num::NonZeroI16`] * [`num::NonZeroI32`] * [`num::NonZeroI64`] * [`num::NonZeroI8`] * [`num::NonZeroIsize`] * [`slice::sort_by_cached_key`] * [`str::escape_debug`] * [`str::escape_default`] * [`str::escape_unicode`] * [`str::split_ascii_whitespace`] #### std * [`Instant::checked_add`] * [`Instant::checked_sub`] * [`SystemTime::checked_add`] * [`SystemTime::checked_sub`] Cargo ----- - [You can now use alternative registries to crates.io.][cargo/6654] Misc ---- - [You can now use the `?` operator in your documentation tests without manually adding `fn main() -> Result<(), _> {}`.][56470] Compatibility Notes ------------------- - [`Command::before_exec` is now deprecated in favor of the unsafe method `Command::pre_exec`.][58059] - [Use of `ATOMIC_{BOOL, ISIZE, USIZE}_INIT` is now deprecated.][57425] As you can now use `const` functions in `static` variables. [58370]: rust-lang/rust#58370 [58406]: rust-lang/rust#58406 [58421]: rust-lang/rust#58421 [58166]: rust-lang/rust#58166 [58044]: rust-lang/rust#58044 [58057]: rust-lang/rust#58057 [58059]: rust-lang/rust#58059 [57809]: rust-lang/rust#57809 [57873]: rust-lang/rust#57873 [57604]: rust-lang/rust#57604 [57367]: rust-lang/rust#57367 [57407]: rust-lang/rust#57407 [57425]: rust-lang/rust#57425 [57106]: rust-lang/rust#57106 [56470]: rust-lang/rust#56470 [cargo/6654]: rust-lang/cargo#6654 [`Any::type_id`]: https://doc.rust-lang.org/std/any/trait.Any.html#tymethod.type_id [`Error::type_id`]: https://doc.rust-lang.org/std/error/trait.Error.html#tymethod.type_id [`atomic::AtomicI16`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI16.html [`atomic::AtomicI32`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI32.html [`atomic::AtomicI64`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI64.html [`atomic::AtomicI8`]: https://doc.rust-lang.org/std/atomic/struct.AtomicI8.html [`atomic::AtomicU16`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU16.html [`atomic::AtomicU32`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU32.html [`atomic::AtomicU64`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU64.html [`atomic::AtomicU8`]: https://doc.rust-lang.org/std/atomic/struct.AtomicU8.html [`convert::Infallible`]: https://doc.rust-lang.org/std/convert/enum.Infallible.html [`convert::TryFrom`]: https://doc.rust-lang.org/std/convert/trait.TryFrom.html [`convert::TryInto`]: https://doc.rust-lang.org/std/convert/trait.TryInto.html [`iter::from_fn`]: https://doc.rust-lang.org/std/iter/fn.from_fn.html [`iter::successors`]: https://doc.rust-lang.org/std/iter/fn.successors.html [`num::NonZeroI128`]: https://doc.rust-lang.org/std/num/struct.NonZeroI128.html [`num::NonZeroI16`]: https://doc.rust-lang.org/std/num/struct.NonZeroI16.html [`num::NonZeroI32`]: https://doc.rust-lang.org/std/num/struct.NonZeroI32.html [`num::NonZeroI64`]: https://doc.rust-lang.org/std/num/struct.NonZeroI64.html [`num::NonZeroI8`]: https://doc.rust-lang.org/std/num/struct.NonZeroI8.html [`num::NonZeroIsize`]: https://doc.rust-lang.org/std/num/struct.NonZeroIsize.html [`slice::sort_by_cached_key`]: https://doc.rust-lang.org/std/slice/fn.sort_by_cached_key [`str::escape_debug`]: https://doc.rust-lang.org/std/primitive.str.html#method.escape_debug [`str::escape_default`]: https://doc.rust-lang.org/std/primitive.str.html#method.escape_default [`str::escape_unicode`]: https://doc.rust-lang.org/std/primitive.str.html#method.escape_unicode [`str::split_ascii_whitespace`]: https://doc.rust-lang.org/std/primitive.str.html#method.split_ascii_whitespace [`Instant::checked_add`]: https://doc.rust-lang.org/std/time/struct.Instant.html#method.checked_add [`Instant::checked_sub`]: https://doc.rust-lang.org/std/time/struct.Instant.html#method.checked_sub [`SystemTime::checked_add`]: https://doc.rust-lang.org/std/time/struct.SystemTime.html#method.checked_add [`SystemTime::checked_sub`]: https://doc.rust-lang.org/std/time/struct.SystemTime.html#method.checked_sub
Notably, hash iterators don't require any trait bounds to be iterated.