Cleanup uses of TypeIdHasher and replace them with StableHasher #50531
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Thanks, @iancormac84! Looks great. Let's see if this affects performance. @bors try |
bors
added a commit
that referenced
this pull request
May 9, 2018
[WIP] Cleanup uses of TypeIdHasher and replace them with StableHasher Fixes #50424 r? @michaelwoerister
|
☀️ Test successful - status-travis |
|
@Mark-Simulacrum perf requested. |
kennytm
added
the
S-waiting-on-perf
|
@Mark-Simulacrum, could you enqueue a perf run please? |
|
Sorry about that, perf is now queued. |
|
Hi @michaelwoerister. Anything I need to do? I noticed the merge conflict, and tried a git rebase, the resolves of which I didn't like, so I cherry-picked my commits on to another branch. I'm still a newbie at using Git, so I'm not 100% sure what's best. |
|
I'm still waiting for the performance results to see if this introduces any regressions. Once those are in and look good, you need to do the following: # get the latest version of master
git fetch upstream
# rebase your branch onto master
git rebase upstream/master
# resolve any conflicts that arise during rebase and require manual resolving
...
# Push your changes so they show up here on github.
git push -fYou can already do that now. Performance testing won't be affected by it. |
|
Ah, okay. Thanks! |
kennytm
removed
the
S-waiting-on-perf
|
Perf result is ready @michaelwoerister. Almost no change in performance in general. |
TimNN
added
S-waiting-on-author
|
@iancormac84: If I understand correctly, this PR should be good to go, could you rebase the PR as requested by @michaelwoerister above? |
iancormac84
changed the title
iancormac84
force-pushed
the
merge-typeidhasher-cleanup
branch
from
2a996a7
to
e44885b
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
☔ The latest upstream changes (presumably #48523) made this pull request unmergeable. Please resolve the merge conflicts. |
iancormac84
force-pushed
the
merge-typeidhasher-cleanup
branch
from
1f595e0
to
14eada5
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Sorry about the delay. I was out sick for a couple of days. Yes, this should be good to go once it passes travis again. |
iancormac84
force-pushed
the
merge-typeidhasher-cleanup
branch
from
14eada5
to
066a4d5
Compare
|
@michaelwoerister sorry to hear. Hope you've made a full recovery. Updates made. |
iancormac84
force-pushed
the
merge-typeidhasher-cleanup
branch
from
066a4d5
to
6f8484b
Compare
kennytm
added
S-waiting-on-review
|
☔ The latest upstream changes (presumably #50615) made this pull request unmergeable. Please resolve the merge conflicts. |
…asher. Also corrected erroneous mention of TypeIdHasher in implementation of HashStable trait.
… with StableHasher.
iancormac84
force-pushed
the
merge-typeidhasher-cleanup
branch
from
6f8484b
to
1839fae
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Thanks for keeping things rebased, @iancormac84! @bors r+ rollup |
|
📌 Commit 0349394 has been approved by |
bors
added
S-waiting-on-bors
kennytm
added a commit
to kennytm/rust
that referenced
this pull request
May 19, 2018
…nup, r=michaelwoerister Cleanup uses of TypeIdHasher and replace them with StableHasher Fixes rust-lang#50424 r? @michaelwoerister
bors
added a commit
that referenced
this pull request
May 19, 2018
Rollup of 8 pull requests Successful merges: - #50531 (Cleanup uses of TypeIdHasher and replace them with StableHasher) - #50819 (Fix potential divide by zero) - #50827 (Update LLVM to 56c931901cfb85cd6f7ed44c7d7520a8de1edf97) - #50829 (CheckLoopVisitor: also visit break expressions) - #50854 (in which the unused shorthand field pattern debacle/saga continues) - #50858 (Reorder description for snippets in rustdoc documentation) - #50883 (Fix warning when building stage0 libcore) - #50889 (Update clippy) Failed merges:
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #50424
r? @michaelwoerister