Conversation
rust-highfive
added
the
S-waiting-on-review
Contributor
Author
|
@bors try |
Collaborator
|
⌛ Trying commit d3bd03ce1d50238d54a70a96b22731cd76cc5e20 with merge 890cc87d1001f65c608c711d7a4d0d48129a43c1... |
Contributor
|
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 |
Collaborator
|
☀️ Try build successful - checks-travis |
Contributor
|
@rust-timer build 890cc87d1001f65c608c711d7a4d0d48129a43c1 |
Collaborator
|
Success: Queued 890cc87d1001f65c608c711d7a4d0d48129a43c1 with parent e305df1, comparison URL. |
Collaborator
|
Finished benchmarking try commit 890cc87d1001f65c608c711d7a4d0d48129a43c1 |
Contributor
Author
|
Seems like this approach works well. There's still some slight memory regressions though. |
Contributor
|
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 |
Xanewok
reviewed
May 1, 2019
src/librustc/hir/map/mod.rs
Outdated
Contributor
There was a problem hiding this comment.
It takes a while to decipher what the structure looks like and why it looks this way - it'd be great if we could add a doc-comment here with an explanation
src/librustc/hir/map/mod.rs
Outdated
Contributor
There was a problem hiding this comment.
It'd be great to also add a comment here and/or rustfmt the function body
Collaborator
|
☔ The latest upstream changes (presumably #60337) made this pull request unmergeable. Please resolve the merge conflicts. |
eddyb
reviewed
May 6, 2019
src/librustc/hir/map/mod.rs
Outdated
Member
There was a problem hiding this comment.
hir::map::Map is the HIR map. Can you rename this type alias to HirEntryMap or something?
eddyb
suggested changes
May 6, 2019
Member
eddyb
left a comment
eddyb
left a comment
There was a problem hiding this comment.
This looks great, r=me modulo nits!
Contributor
Author
|
@bors r=eddyb |
Collaborator
|
📌 Commit d33db6e has been approved by |
bors
added
S-waiting-on-bors
Collaborator
Collaborator
|
☀️ Test successful - checks-travis, status-appveyor |
Merged
bjorn3
reviewed
May 8, 2019
| return Some(idx) | ||
| } | ||
| } | ||
| fn matces_suffix(&self, hir: HirId) -> bool { |
| } | ||
| }; | ||
|
|
||
| self.all_ids().filter(move |hir| nodes.matces_suffix(*hir)).map(move |hir| { |
Member
|
I am a little confused about the order of comments on this PR: bors merges (1h ago) -> me reviewing it (few min ago) -> Zoxc gives r+ (3h ago). |
Contributor
Author
|
@bjorn3 It's github new feature, time-travelling comments. |
Member
|
When looking at my notifications list, it said bors was the last commenter :) |
Contributor
Author
|
You can see that I will post my comment in 3 hours. Github simply sorts comments by time! |
Member
|
On mobile it says 2 hours ago though. |
Contributor
Author
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Builds on #59042
cc @ljedrz
r? @eddyb