Fix ordering for default impl check in the new solver

[nnethercote]

The second commit of #160605 moved the default impl check later, for better performance, which introduced a regression. This commit moves the check a little earlier, so it is after the args_may_unify call (thus retaining the perf benefit) but before the probe_trait_candidate (which has side-effects).

The check is now duplicated in three GoalKind::consider_impl_candidate methods, which is unfortunate, but it fits in with the existing duplicated code in those methods. And it means another copy of the check (in try_assemble_bounds_via_registered_opaques) can be removed.

Fixes #160994.

r? @lcnr

[rustbot]

⚠️ Warning ⚠️

• There are issue links (such as #123) in the commit messages of the following commits.
  Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.
  • 39b6dbe

[nnethercote]

LLM disclosure: an LLM helped with analysis and review of this PR. I wrote all the code and text myself.

[nnethercote]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 71c4692 (71c4692a099ddb94f11b4c9e214701c81b4f8803)
Base parent: 8fa1c96 (8fa1c96cfd489e4c27654c144ae871ce2c4db6c6)

[rust-timer]

Finished benchmarking commit (71c4692): comparison URL.

Overall result: ❌ regressions - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.8%	[0.5%, 1.2%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Cycles

This perf run didn't have relevant results for this metric.

Binary size

Results (primary -0.0%, secondary -0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.0%	[-0.0%, -0.0%]	4
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	41
All ❌✅ (primary)	-0.0%	[-0.0%, -0.0%]	4

Bootstrap: 458.344s -> 457.305s (-0.23%)
Artifact size: 398.95 MiB -> 398.92 MiB (-0.01%)

[nnethercote]

A slight perf regression, but needed for correctness, and it's only a small fraction of the improvement from #160605.

@rustbot label: +perf-regression-triaged

[lcnr]

one other option: could we change the for_each_relevant_impl and for_each_blanket_impl queries to only return non-default impls and have a separate for_each_default_impl?

r=me on this change itself, even if I quite dislike default impls to negatively impact perf here

View changes since this review