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 6 pull requests #79017

Merged
merged 17 commits into from
Nov 13, 2020
Merged

Rollup of 6 pull requests #79017

merged 17 commits into from
Nov 13, 2020

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

tkaitchuck and others added 17 commits October 15, 2020 14:03
…ve hasher.

Signed-off-by: Tom Kaitchuck <tom.kaitchuck@emc.com>
Signed-off-by: Tom Kaitchuck <Tom.Kaitchuck@gmail.com>
Co-authored-by: Mara Bos <m-ou.se@m-ou.se>
…r=pnkfelix

Add regression test for issue rust-lang#76042

Originally posted in rust-lang#76042 (comment).
r? `@pnkfelix`
Doc change: Remove mention of `fnv` in HashMap

Disclaimer: I am the author of [aHash](https://github.com/tkaitchuck/aHash).

This changes the Rustdoc in `HashMap` from mentioning the `fnv` crate to mentioning the `aHash` crate, as an alternative `Hasher` implementation.

### Why

Fnv [has poor hash quality](https://github.com/rurban/smhasher), is [slow for larger keys](https://github.com/tkaitchuck/aHash/blob/master/compare/readme.md#speed), and does not provide dos resistance, because it is unkeyed (this can also cause [other problems](https://accidentallyquadratic.tumblr.com/post/153545455987/rust-hash-iteration-reinsertion)).

Fnv has acceptable performance for integers and has very poor performance with keys >32 bytes. This is the reason it was removed from the standard library in rust-lang#37229 .

Because regardless of which dimension you value, there are better alternatives, it does not make sense for anyone to consider using `fnv`.

The text mentioning `fnv` in the standard library continues to create confusion: rust-lang/hashbrown#153  rust-lang/hashbrown#9 . There are also a number of [crates using it](https://crates.io/crates/fnv/reverse_dependencies) a great many of which are hashing strings (Which is when Fnv is the [worst](https://github.com/cbreeden/fxhash#benchmarks), [possible](https://github.com/tkaitchuck/aHash#speed), [choice](http://cglab.ca/~abeinges/blah/hash-rs/).)

I think aHash makes the most sense to mention as an alternative because it is the most credible option (in my obviously biased opinion). It offers [good performance on numbers and strings](https://github.com/tkaitchuck/aHash/blob/master/compare/readme.md#speed), is [of high quality](https://github.com/tkaitchuck/aHash#hash-quality), and [provides dos resistance](https://github.com/tkaitchuck/aHash/wiki/How-aHash-is-resists-DOS-attacks). It is popular (see [stats](https://crates.io/crates/ahash)) and is the default hasher for [hashbrown](https://crates.io/crates/hashbrown) and [dashmap](https://crates.io/crates/dashmap) which are the most popular alternative hashmaps. Finally it does not have any of the [`gotcha` cases](https://github.com/tkaitchuck/aHash#fxhash) that `FxHash` suffers from. (Which is the other popular hashing option when DOS attacks are not a concern)

Signed-off-by: Tom Kaitchuck <tom.kaitchuck@emc.com>
…sakis

Add type to `ConstKind::Placeholder`

I simply threaded `<'tcx>` through everything that required it. I'm not sure whether this is the correct thing to do, but it seems to work.

r? `@nikomatsakis`
…, r=jyn514

Rustdoc check option

The ultimate goal behind this option would be to have `rustdoc --check` being run when you use `cargo check` as a second step.

r? `@jyn514`
add dropck test for const params

r? `@nikomatsakis` or `@varkor`
add explicit test for const param promotion

r? `@RalfJung`
@rustbot rustbot added the rollup A PR which is a rollup label Nov 13, 2020
@GuillaumeGomez
Copy link
Member Author

@bors: r+ p=6

@bors
Copy link
Contributor

bors commented Nov 13, 2020

📌 Commit 7ea8e32 has been approved by GuillaumeGomez

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 13, 2020
@bors
Copy link
Contributor

bors commented Nov 13, 2020

⌛ Testing commit 7ea8e32 with merge f2a11a2...

@bors
Copy link
Contributor

bors commented Nov 13, 2020

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing f2a11a2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 13, 2020
@bors bors merged commit f2a11a2 into rust-lang:master Nov 13, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 13, 2020
@GuillaumeGomez GuillaumeGomez deleted the rollup-5orhudd branch November 13, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants