Skip to content
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

Rollup of 2 pull requests #41312

Merged
merged 14 commits into from
Apr 15, 2017
Merged

Rollup of 2 pull requests #41312

merged 14 commits into from
Apr 15, 2017

Conversation

frewsxcv
Copy link
Member

chordowl and others added 14 commits April 6, 2017 21:59
Part of rust-lang#29357.
* split summary and explanation more clearly
* added link to nomicon for "zero-sized"
* "does not need construction" -> say how it can be created, and that it
  doesn't need to be done with `HashMap` or `HashSet`
Part of rust-lang#29357.
* split summary and explanation more clearly, while expanding the
  explanation to make the reason for `BuildHasher` existing more clear
* added an example illustrating that `Hasher`s created by one `BuildHasher`
  should be identical
* added links
* repeated the fact that hashers produced should be identical in
  `build_hasher`s method docs
Part of rust-lang#29357.
* merged "Derivable" and "How can I implement `Hash`?" sections into one
  "Implementing `Hash`" section to aid coherency
* added an example for `#[derive(Hash)]`
* moved part about relation between `Hash` and `Eq` into a new "`Hash` and
  `Eq`" section; changed wording to be more consistent with `HashMap` and
  `HashSet`'s
* explicitly mentioned `#[derive(PartialEq, Eq, Hash)]` in the new section
* changed method summaries for consistency, adding links and examples
Part of rust-lang#29357.
* rephrased summary sentences to be less redundant
* expanded top-level docs, adding a usage example and links to relevant
  methods (`finish`, `write` etc) as well as `Hash`
* added examples to the `finish` and `write` methods
This patch allows overlap to occur between any two impls of a trait for
traits which have no associated items.

Several compile-fail tests around coherence had to be changed to add at
least one item to the trait they test against.

Ref rust-lang#29864
I've added some explicit tests that negative impls are allowed to
overlap, and also to make sure that the feature doesn't interfere with
specialization. I've not added an explicit test for positive overlapping
with negative, as that's already tested elsewhere.
Improve std::hash docs

Fixes rust-lang#29357.

For details on what exactly I've done, see the commit descriptions.

There are some things I'm not sure about, but would like to address before merging this so the issue can be closed; any feedback on these points would really be appriciated:
* [x] ~I didn't touch the module level docs at all. On the one hand, I think they could use a short overview over the module; on the other hand, the module really isn't that big and I don't know if I could really do anything beyond just duplicating the type's summaries...~
* [x] ~I feel like the module-level examples are quite long-winded and not to the point, but I couldn't really think of anything better. Any ideas?~
* [x] ~Should `Hasher` get an example for implementing it? There is one in the module documentation, but it only "implements" it via `unimplemented!` and I'm not sure what the value of that is.~
* [x] ~Should `Hasher`'s `write_{int}` methods get examples?~

If there's anything else you'd like to see in std::hash's docs, please let me know!

r? @rust-lang/docs
…ikomatsakis

Implement RFC 1268.

Rebased version of rust-lang#40097.

Tracking issue: rust-lang#29864.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@frewsxcv
Copy link
Member Author

@bors r+ p=10

@bors
Copy link
Contributor

bors commented Apr 15, 2017

📌 Commit 3adcd1c has been approved by frewsxcv

@bors
Copy link
Contributor

bors commented Apr 15, 2017

⌛ Testing commit 3adcd1c with merge be1a74e...

bors added a commit that referenced this pull request Apr 15, 2017
Rollup of 2 pull requests

- Successful merges: #41125, #41309
- Failed merges:
@bors
Copy link
Contributor

bors commented Apr 15, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: frewsxcv
Pushing be1a74e to master...

@bors bors merged commit 3adcd1c into rust-lang:master Apr 15, 2017
@bors bors mentioned this pull request Apr 15, 2017
3 tasks
@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants