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

TypeIdHasher: hash usize as u64 #36795

Merged
merged 1 commit into from
Sep 28, 2016
Merged

TypeIdHasher: hash usize as u64 #36795

merged 1 commit into from
Sep 28, 2016

Conversation

TimNN
Copy link
Contributor

@TimNN TimNN commented Sep 28, 2016

Fixes #36793.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -436,7 +436,7 @@ impl<'a, 'gcx, 'tcx, H: Hasher> TypeVisitor<'tcx> for TypeIdHasher<'a, 'gcx, 'tc
TyInt(i) => self.hash(i),
TyUint(u) => self.hash(u),
TyFloat(f) => self.hash(f),
TyArray(_, n) => self.hash(n),
TyArray(_, n) => self.hash(n as u64),
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch! Wait, why do we even do this?! TyArray should've held u64 from the beginning.

@eddyb
Copy link
Member

eddyb commented Sep 28, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Sep 28, 2016

📌 Commit 4053af9 has been approved by eddyb

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 28, 2016
@alexcrichton
Copy link
Member

@bors: p=1

(fixing a regression)

Thanks @TimNN!

@bors
Copy link
Contributor

bors commented Sep 28, 2016

⌛ Testing commit 4053af9 with merge f17b1e0...

bors added a commit that referenced this pull request Sep 28, 2016
TypeIdHasher: hash usize as u64

Fixes #36793.
bors added a commit that referenced this pull request Sep 28, 2016
Rollup of 11 pull requests

- Successful merges: #36376, #36672, #36740, #36757, #36765, #36769, #36782, #36783, #36784, #36795, #36796
- Failed merges:
@bors bors merged commit 4053af9 into rust-lang:master Sep 28, 2016
@eddyb eddyb added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 29, 2016
@eddyb
Copy link
Member

eddyb commented Sep 29, 2016

Nominating for backport to 1.13 beta on the basis of #36793 (comment) (needed by stage0 cross-compile).

@TimNN TimNN deleted the fix-36793 branch September 30, 2016 09:40
@alexcrichton
Copy link
Member

Regarding the nomination here, unfortunately it looks like this wasn't enough to fix #36793 but the support in #36866 I believe can be backported independently from this.

@brson brson added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 5, 2016
@nikomatsakis
Copy link
Contributor

@alexcrichton are you saying this should not be backported, or just that it is not itself enough and we must backport #36866 too?

@alexcrichton
Copy link
Member

@nikomatsakis I believe this should not be backported as #36866 ended up undoing it after more cases of hashing usize were discovered.

@nikomatsakis
Copy link
Contributor

@alexcrichton ok.

@nikomatsakis nikomatsakis removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 12, 2016
@nikomatsakis
Copy link
Contributor

Based on @alexcrichton's comments, decided not to backport this PR.

cc @rust-lang/compiler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants