Simplify `ImplSource` candidates a bit #112687

compiler-errors · 2023-06-16T01:20:23Z

Reduce the number of impl source candidates, which will hopefully make our life easier when trying to adapt things like SelectionContext::select and codegen_select_candidate for the new solver.

r? @lcnr but feel free to reassign

lcnr

some nits, then r=me

compiler/rustc_ty_utils/src/instance.rs

lcnr · 2023-06-16T07:48:38Z

@bors try @rust-timer queue

bors · 2023-06-16T07:48:47Z

⌛ Trying commit f739157811023d2fbfbae26a9b40ef826b4e6dca with merge feb4cc8363d70e29b6c7437d12f17c0a77733a40...

bors · 2023-06-16T09:05:52Z

☀️ Try build successful - checks-actions
Build commit: feb4cc8363d70e29b6c7437d12f17c0a77733a40 (feb4cc8363d70e29b6c7437d12f17c0a77733a40)

rust-timer · 2023-06-16T11:10:53Z

Finished benchmarking commit (feb4cc8363d70e29b6c7437d12f17c0a77733a40): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -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)	-0.3%	[-0.3%, -0.2%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-0.3%, -0.2%]	2

Max RSS (memory usage)

Results

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)	-	-	0
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	2
All ❌✅ (primary)	-0.8%	[-0.8%, -0.8%]	1

Cycles

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

Binary size

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

Bootstrap: 647.175s -> 647.961s (0.12%)

compiler-errors · 2023-06-16T18:28:39Z

Removed the debug assertions, let's do perf one more time to see if it really matters. Otherwise, I'll just add em back.

@bors try @rust-timer queue

bors · 2023-06-16T18:28:49Z

⌛ Trying commit 10b21d2a8fb4cacc0252810f3793cf987194bfcb with merge 960e730ef62efb32f3f9d522cae517e73a226126...

bors · 2023-06-16T20:02:59Z

☀️ Try build successful - checks-actions
Build commit: 960e730ef62efb32f3f9d522cae517e73a226126 (960e730ef62efb32f3f9d522cae517e73a226126)

rust-timer · 2023-06-16T22:09:11Z

Finished benchmarking commit (960e730ef62efb32f3f9d522cae517e73a226126): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +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)	1.3%	[1.3%, 1.4%]	3
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.2%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-0.3%, -0.2%]	3

Max RSS (memory usage)

Results

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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-3.3%, -2.8%]	3
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 646.921s -> 646.646s (-0.04%)

compiler-errors · 2023-06-16T22:11:38Z

Not sure if that's just noise or what, but I'm gonna add the cfg back.

@bors r=lcnr

bors · 2023-06-16T22:11:40Z

📌 Commit 12ac93754627dc4933069ceaa833de761dd418c4 has been approved by lcnr

It is now in the queue for this repository.

bors · 2023-06-17T03:28:47Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout simplify-impl-source (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self simplify-impl-source --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Auto-merging compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Auto-merging compiler/rustc_trait_selection/src/traits/project.rs
CONFLICT (content): Merge conflict in compiler/rustc_trait_selection/src/traits/project.rs
Auto-merging compiler/rustc_middle/src/traits/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

compiler-errors · 2023-06-17T04:15:26Z

@bors r=lcnr

bors · 2023-06-17T04:15:28Z

📌 Commit d97d4eb has been approved by lcnr

It is now in the queue for this repository.

bors · 2023-06-17T06:26:58Z

⌛ Testing commit d97d4eb with merge f90d57d...

bors · 2023-06-17T09:04:31Z

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

rust-timer · 2023-06-17T11:02:54Z

Finished benchmarking commit (f90d57d): comparison URL.

Overall result: ❌✅ regressions and 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)	2.0%	[2.0%, 2.0%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[-0.3%, 2.0%]	2

Max RSS (memory usage)

Results

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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-3.4%, -2.2%]	2
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 656.42s -> 656.303s (-0.02%)

Patches: * rust-lang/rust#112687

Don't ICE when auto trait has assoc ty in old solver Kinda a pointless change to make, but it's observable w/o the feature gate, so let's just fix it. I reintroduced this ICE when I removed the "auto impl" kind from `ImplSource` in rust-lang#112687. Fixes rust-lang#117829 Fixes rust-lang#127746

Rollup merge of rust-lang#128160 - compiler-errors:auto, r=jackh726 Don't ICE when auto trait has assoc ty in old solver Kinda a pointless change to make, but it's observable w/o the feature gate, so let's just fix it. I reintroduced this ICE when I removed the "auto impl" kind from `ImplSource` in rust-lang#112687. Fixes rust-lang#117829 Fixes rust-lang#127746

rustbot assigned lcnr Jun 16, 2023

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 Jun 16, 2023

lcnr reviewed Jun 16, 2023

View reviewed changes

compiler/rustc_ty_utils/src/instance.rs Outdated Show resolved Hide resolved

compiler/rustc_ty_utils/src/instance.rs Show resolved Hide resolved

compiler/rustc_ty_utils/src/instance.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 16, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 16, 2023

rust-cloud-vms bot force-pushed the simplify-impl-source branch from f739157 to 10b21d2 Compare June 16, 2023 18:28

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 16, 2023

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 16, 2023

rust-cloud-vms bot force-pushed the simplify-impl-source branch 2 times, most recently from e158726 to 12ac937 Compare June 16, 2023 22:11

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 Jun 16, 2023

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 17, 2023

compiler-errors added 5 commits June 17, 2023 03:32

Remove some ImplSource candidates

1704481

Simplify some impl source candidates

9e68b6f

Simplify an ObjectData field

1311bb5

Simplify even more candidates

2835d9d

Remove even more redundant builtin candidates

d97d4eb

compiler-errors force-pushed the simplify-impl-source branch from 12ac937 to d97d4eb Compare June 17, 2023 04:15

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 17, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 17, 2023

bors merged commit f90d57d into rust-lang:master Jun 17, 2023

rustbot added this to the 1.72.0 milestone Jun 17, 2023

rustbot removed the perf-regression Performance regression. label Jun 17, 2023

floriangru mentioned this pull request Jun 26, 2023

Update to toolchain 2023-06-29 creusot-rs/creusot#819

Merged

floriangru added a commit to floriangru/creusot that referenced this pull request Jun 27, 2023

2/3 Updating for nightly-2023-06-25

2bece4a

Patches: * rust-lang/rust#112687

compiler-errors deleted the simplify-impl-source branch August 11, 2023 20:19

matthiaskrgr mentioned this pull request Nov 12, 2023

ice: auto traits: unexpected builtin trait with associated type #117829

Closed

compiler-errors mentioned this pull request Jul 24, 2024

Don't ICE when auto trait has assoc ty in old solver #128160

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify `ImplSource` candidates a bit #112687

Simplify `ImplSource` candidates a bit #112687

compiler-errors commented Jun 16, 2023

lcnr left a comment

lcnr commented Jun 16, 2023

This comment has been minimized.

bors commented Jun 16, 2023

bors commented Jun 16, 2023

This comment has been minimized.

rust-timer commented Jun 16, 2023

compiler-errors commented Jun 16, 2023

This comment has been minimized.

bors commented Jun 16, 2023

bors commented Jun 16, 2023

This comment has been minimized.

rust-timer commented Jun 16, 2023

compiler-errors commented Jun 16, 2023

bors commented Jun 16, 2023

bors commented Jun 17, 2023

compiler-errors commented Jun 17, 2023

bors commented Jun 17, 2023

bors commented Jun 17, 2023

bors commented Jun 17, 2023

rust-timer commented Jun 17, 2023

Simplify ImplSource candidates a bit #112687

Simplify ImplSource candidates a bit #112687

Conversation

compiler-errors commented Jun 16, 2023

lcnr left a comment

Choose a reason for hiding this comment

lcnr commented Jun 16, 2023

This comment has been minimized.

bors commented Jun 16, 2023

bors commented Jun 16, 2023

This comment has been minimized.

rust-timer commented Jun 16, 2023

Overall result: ✅ improvements - no action needed

compiler-errors commented Jun 16, 2023

This comment has been minimized.

bors commented Jun 16, 2023

bors commented Jun 16, 2023

This comment has been minimized.

rust-timer commented Jun 16, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

compiler-errors commented Jun 16, 2023

bors commented Jun 16, 2023

bors commented Jun 17, 2023

compiler-errors commented Jun 17, 2023

bors commented Jun 17, 2023

bors commented Jun 17, 2023

bors commented Jun 17, 2023

rust-timer commented Jun 17, 2023

Overall result: ❌✅ regressions and improvements - no action needed

Simplify `ImplSource` candidates a bit #112687

Simplify `ImplSource` candidates a bit #112687