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

Properly hash enums #4066

Merged
merged 4 commits into from
May 15, 2019
Merged

Properly hash enums #4066

merged 4 commits into from
May 15, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 7, 2019

While I wrote this I was saved by a clippy lint... I accidentally fetched the discriminant of a reference to an enum and not of an enum.

changelog: reduce hash collisions during clippy-internal hashing

return e.hash(&mut self.s);
}

std::mem::discriminant(&e.node).hash(&mut self.s);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use mem::discriminant we don't need the let c: fn stuff below

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 8, 2019

📌 Commit b30c6dc has been approved by Manishearth

bors added a commit that referenced this pull request May 8, 2019
Properly hash enums

While I wrote this I was saved by a clippy lint... I accidentally fetched the discriminant of a reference to an enum and not of an enum.

changelog: reduce hash collisions during clippy-internal hashing
@bors
Copy link
Collaborator

bors commented May 8, 2019

⌛ Testing commit b30c6dc with merge 849c2c0...

@bors
Copy link
Collaborator

bors commented May 8, 2019

💔 Test failed - status-appveyor

@bors
Copy link
Collaborator

bors commented May 11, 2019

☔ The latest upstream changes (presumably #4080) made this pull request unmergeable. Please resolve the merge conflicts.

@@ -635,24 +580,15 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
pub fn hash_stmt(&mut self, b: &Stmt) {
match b.node {
StmtKind::Local(ref local) => {
let c: fn(_) -> _ = StmtKind::Local;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By removing this, the discriminant needs to be hashed at the beginning of the function. I'll add this.

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 14, 2019
@oli-obk
Copy link
Contributor Author

oli-obk commented May 14, 2019

@bors r=Manishearth

@bors
Copy link
Collaborator

bors commented May 14, 2019

📌 Commit 5dea5d4 has been approved by Manishearth

bors added a commit that referenced this pull request May 14, 2019
Properly hash enums

While I wrote this I was saved by a clippy lint... I accidentally fetched the discriminant of a reference to an enum and not of an enum.

changelog: reduce hash collisions during clippy-internal hashing
@bors
Copy link
Collaborator

bors commented May 14, 2019

⌛ Testing commit 5dea5d4 with merge b6abfba...

@phansch
Copy link
Member

phansch commented May 15, 2019

Looks like bors did not merge the PR?

@bors retry

@bors
Copy link
Collaborator

bors commented May 15, 2019

⌛ Testing commit 5dea5d4 with merge f49d878...

bors added a commit that referenced this pull request May 15, 2019
Properly hash enums

While I wrote this I was saved by a clippy lint... I accidentally fetched the discriminant of a reference to an enum and not of an enum.

changelog: reduce hash collisions during clippy-internal hashing
@mati865
Copy link
Contributor

mati865 commented May 15, 2019

Bors was redeployed yesterday, maybe timing wasn't good.

@bors
Copy link
Collaborator

bors commented May 15, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Manishearth
Pushing f49d878 to master...

@bors bors merged commit 5dea5d4 into master May 15, 2019
@flip1995 flip1995 deleted the hash branch May 15, 2019 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants