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

Remove unnecessary impl sorting in queries and metadata #120812

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Feb 8, 2024

Removes unnecessary impl sorting because queries already return their keys in HIR definition order: #120371 (comment)

r? @cjgillot or @lcnr -- unless I totally misunderstood what was being asked for here? 😆

fixes #120371

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 8, 2024
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Feb 8, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

@cjgillot
Copy link
Contributor

cjgillot commented Feb 8, 2024

I need to check something before approving. I'll get back to you some time next week.

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member Author

will fix later

@bors
Copy link
Contributor

bors commented Feb 28, 2024

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

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member Author

@cjgillot: Did you have an opportunity to investigate this?

@bors
Copy link
Contributor

bors commented Apr 30, 2024

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

@wesleywiser
Copy link
Member

Hi @cjgillot, just wanted to see if you had any more feedback for @compiler-errors 🙂

@wesleywiser
Copy link
Member

r? rust-lang/compiler

@rustbot rustbot assigned TaKO8Ki and unassigned cjgillot Jul 18, 2024
@compiler-errors
Copy link
Member Author

TaKO8Ki hasn't reviewed code for a month, and also I don't believe they're familiar with metadata encoding. I'm going to preemptively re-roll.

r? compiler

@rustbot rustbot assigned lcnr and unassigned TaKO8Ki Jul 18, 2024
@lcnr
Copy link
Contributor

lcnr commented Jul 18, 2024

r=me after rebase. We discussed this on zulip previously where i looked into the code. I am fairly confident that they are already sorted and this can always be reverted/solved differently going forward

@compiler-errors
Copy link
Member Author

@bors r=lcnr rollup=never

@bors
Copy link
Contributor

bors commented Jul 21, 2024

📌 Commit e0ba193 has been approved by lcnr

It is now in the queue for this repository.

@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 Jul 21, 2024
@bors
Copy link
Contributor

bors commented Jul 21, 2024

⌛ Testing commit e0ba193 with merge 2a5e296...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2024
Remove unnecessary impl sorting in queries and metadata

Removes unnecessary impl sorting because queries already return their keys in HIR definition order: rust-lang#120371 (comment)

r? `@cjgillot` or `@lcnr` -- unless I totally misunderstood what was being asked for here? 😆

fixes rust-lang#120371
@Mark-Simulacrum
Copy link
Member

@bors retry -- yielding to stable release

@bors
Copy link
Contributor

bors commented Jul 21, 2024

⌛ Testing commit e0ba193 with merge 0f8534e...

@bors
Copy link
Contributor

bors commented Jul 22, 2024

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 0f8534e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 22, 2024
@bors bors merged commit 0f8534e into rust-lang:master Jul 22, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Jul 22, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0f8534e): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.2% [-2.1%, -0.4%] 2
Improvements ✅
(secondary)
-0.3% [-0.4%, -0.3%] 2
All ❌✅ (primary) -1.2% [-2.1%, -0.4%] 2

Max RSS (memory usage)

Results (secondary 4.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.8% [4.2%, 5.8%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results (primary -1.4%, secondary -3.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.8% [3.8%, 3.8%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.9% [-3.6%, -1.1%] 12
Improvements ✅
(secondary)
-3.2% [-3.7%, -2.5%] 13
All ❌✅ (primary) -1.4% [-3.6%, 3.8%] 13

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 772.021s -> 771.951s (-0.01%)
Artifact size: 328.86 MiB -> 328.89 MiB (0.01%)

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 25, 2024
Remove unnecessary impl sorting in queries and metadata

Removes unnecessary impl sorting because queries already return their keys in HIR definition order: rust-lang#120371 (comment)

r? `@cjgillot` or `@lcnr` -- unless I totally misunderstood what was being asked for here? 😆

fixes rust-lang#120371
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. 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.

the order of tcx.implementations_of_trait is unstable
10 participants