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

Avoid sorting by DefId for necessary_variants() #83599

Merged
merged 1 commit into from
Apr 3, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 28, 2021

Follow-up to #83074. Originally I tried removing impl Ord for DefId but that hit lots of errors 😅 so I thought I would start with easy things.

I am not sure whether this could actually cause invalid query results, but this is used from MarkSymbolVisitor::visit_arm so it's at least feasible.

r? @Aaron1011

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2021
@rust-log-analyzer

This comment has been minimized.

@Aaron1011
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 2, 2021

📌 Commit a0957c9 has been approved by Aaron1011

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 2, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 2, 2021
Avoid sorting by DefId for `necessary_variants()`

Follow-up to rust-lang#83074. Originally I tried removing `impl Ord for DefId` but that hit *lots* of errors 😅 so I thought I would start with easy things.

I am not sure whether this could actually cause invalid query results, but this is used from `MarkSymbolVisitor::visit_arm` so it's at least feasible.

r? `@Aaron1011`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 3, 2021
Avoid sorting by DefId for `necessary_variants()`

Follow-up to rust-lang#83074. Originally I tried removing `impl Ord for DefId` but that hit *lots* of errors 😅 so I thought I would start with easy things.

I am not sure whether this could actually cause invalid query results, but this is used from `MarkSymbolVisitor::visit_arm` so it's at least feasible.

r? ``@Aaron1011``
@bors
Copy link
Contributor

bors commented Apr 3, 2021

⌛ Testing commit a0957c9 with merge cb17136...

@bors
Copy link
Contributor

bors commented Apr 3, 2021

☀️ Test successful - checks-actions
Approved by: Aaron1011
Pushing cb17136 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 3, 2021
@bors bors merged commit cb17136 into rust-lang:master Apr 3, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 3, 2021
@klensy
Copy link
Contributor

klensy commented Apr 3, 2021

Isn't FxIndexSet preserve insertion order, but FxHashSet only use custom hasher?

Ah, hashset used only to check duplicates, my bad.

@jyn514 jyn514 deleted the unorderable branch April 3, 2021 12:41
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. 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