Skip to content

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
…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 <[email protected]>
…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
Collaborator

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
Collaborator

bors commented Nov 13, 2020

⌛ Testing commit 7ea8e32 with merge f2a11a2...

@bors
Copy link
Collaborator

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