unify query canonicalization mode #118968

aliemjay · 2023-12-15T06:12:12Z

Exclude from canonicalization only the static lifetimes that appear in the param env because of #118965 . Any other occurrence can be canonicalized safely AFAICT.

r? @lcnr

lcnr · 2023-12-15T13:20:55Z

happy with this. I want to think about a way fix #118965 and completely remove the 'static special case, but this is a step in the right direction. Let's make sure there's no other dependency on the static specialcase.

@bors try

bors · 2023-12-15T13:22:06Z

⌛ Trying commit 707c4f9 with merge 7e4f74f...

unify query canonicalization mode Exclude from canonicalization only the static lifetimes that appear in the param env because of rust-lang#118965 . Any other occurrence can be canonicalized safely AFAICT. r? `@lcnr`

aliemjay · 2023-12-15T13:35:00Z

compiler/rustc_infer/src/infer/canonical/canonicalizer.rs

+            query_state,
+            |tcx, param_env, query_state| {
+                // FIXME(#118965): We don't canonicalize the static lifetimes that appear in the
+                // `param_env` beacause they are treated differently by trait selection.


Suggested change

// `param_env` beacause they are treated differently by trait selection.

// `param_env` because they are treated differently by trait selection.

bors · 2023-12-15T14:51:07Z

☀️ Try build successful - checks-actions
Build commit: 7e4f74f (7e4f74fb8736fea8f455bb6e094122965ae8f4cd)

lcnr · 2023-12-15T17:14:34Z

@craterbot check

craterbot · 2023-12-15T17:14:41Z

👌 Experiment pr-118968 created and queued.
🤖 Automatically detected try build 7e4f74f
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

aliemjay · 2023-12-18T04:35:45Z

@rust-timer build 7e4f74f

rust-timer · 2023-12-18T06:44:28Z

Finished benchmarking commit (7e4f74f): 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.2%	[-0.2%, -0.2%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.2%	[-0.2%, -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)	2.0%	[1.0%, 3.0%]	4
Regressions ❌ (secondary)	3.6%	[2.7%, 4.9%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-2.2%, -1.1%]	4
All ❌✅ (primary)	2.0%	[1.0%, 3.0%]	4

Cycles

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.6%	[-0.6%, -0.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.6%	[-0.6%, -0.6%]	1

Binary size

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

Bootstrap: 671.047s -> 671.504s (0.07%)
Artifact size: 312.39 MiB -> 312.40 MiB (0.00%)

craterbot · 2023-12-25T19:32:09Z

🚧 Experiment pr-118968 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

aliemjay · 2023-12-27T12:45:04Z

@craterbot check p=1 crates=https://crater-reports.s3.amazonaws.com/pr-118968/retry-regressed-list.txt

craterbot · 2023-12-27T12:45:09Z

👌 Experiment pr-118968-1 created and queued.
🤖 Automatically detected try build 7e4f74f
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2023-12-27T15:41:53Z

🎉 Experiment pr-118968 is completed!
📊 25 regressed and 4 fixed (399777 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2023-12-27T16:02:15Z

🚧 Experiment pr-118968-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2023-12-27T16:12:24Z

🎉 Experiment pr-118968-1 is completed!
📊 0 regressed and 0 fixed (25 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

lcnr · 2024-01-08T16:20:41Z

cc @rust-lang/types this PR changes the old canonicalization to always erase 'static unless it is in of the param_env. We have to keep 'static in the param_env itself due to #118965 in the old solver.

I don't think 'static impacts the solver in any other way after #102472, and I believe this to be a another step in the right direction. Given that this shouldn't impact the behavior in any way I am going to go ahead and merge this.

@bors r+ rollup=never

bors · 2024-01-08T16:20:44Z

📌 Commit 707c4f9 has been approved by lcnr

It is now in the queue for this repository.

bors · 2024-01-09T09:20:37Z

⌛ Testing commit 707c4f9 with merge be00c5a...

bors · 2024-01-09T11:18:27Z

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

rust-timer · 2024-01-09T12:32:50Z

Finished benchmarking commit (be00c5a): 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)	-	-	0
Regressions ❌ (secondary)	0.5%	[0.5%, 0.5%]	1
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.2%	[-0.2%, -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)	2.1%	[0.5%, 4.2%]	7
Regressions ❌ (secondary)	2.0%	[1.0%, 3.1%]	3
Improvements ✅ (primary)	-1.6%	[-1.6%, -1.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[-1.6%, 4.2%]	8

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: 669.888s -> 669.477s (-0.06%)
Artifact size: 308.54 MiB -> 308.51 MiB (-0.01%)

rustbot assigned lcnr Dec 15, 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 Dec 15, 2023

This comment has been minimized.

Sign in to view

unify query canonicalization mode

707c4f9

rust-cloud-vms bot force-pushed the canon-static branch from d113f8a to 707c4f9 Compare December 15, 2023 06:59

aliemjay commented Dec 15, 2023

View reviewed changes

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2023

This comment was marked as outdated.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 18, 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 Dec 18, 2023

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Dec 27, 2023

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 Jan 8, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 9, 2024

bors merged commit be00c5a into rust-lang:master Jan 9, 2024
12 checks passed

rustbot added this to the 1.77.0 milestone Jan 9, 2024

bors mentioned this pull request Jan 9, 2024

error on incorrect implied bounds in wfcheck except for Bevy dependents #118553

Merged

aliemjay deleted the canon-static branch March 30, 2024 01:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unify query canonicalization mode #118968

unify query canonicalization mode #118968

aliemjay commented Dec 15, 2023

This comment has been minimized.

lcnr commented Dec 15, 2023

bors commented Dec 15, 2023

aliemjay Dec 15, 2023

bors commented Dec 15, 2023

lcnr commented Dec 15, 2023

craterbot commented Dec 15, 2023

aliemjay commented Dec 18, 2023 •

edited

Loading

This comment was marked as outdated.

This comment has been minimized.

rust-timer commented Dec 18, 2023

craterbot commented Dec 25, 2023

aliemjay commented Dec 27, 2023

craterbot commented Dec 27, 2023

craterbot commented Dec 27, 2023

craterbot commented Dec 27, 2023

craterbot commented Dec 27, 2023

lcnr commented Jan 8, 2024

bors commented Jan 8, 2024

bors commented Jan 9, 2024

bors commented Jan 9, 2024

rust-timer commented Jan 9, 2024

	// `param_env` beacause they are treated differently by trait selection.
	// `param_env` because they are treated differently by trait selection.

unify query canonicalization mode #118968

unify query canonicalization mode #118968

Conversation

aliemjay commented Dec 15, 2023

This comment has been minimized.

lcnr commented Dec 15, 2023

bors commented Dec 15, 2023

Choose a reason for hiding this comment

bors commented Dec 15, 2023

lcnr commented Dec 15, 2023

craterbot commented Dec 15, 2023

aliemjay commented Dec 18, 2023 • edited Loading

This comment was marked as outdated.

This comment has been minimized.

rust-timer commented Dec 18, 2023

Overall result: ✅ improvements - no action needed

craterbot commented Dec 25, 2023

aliemjay commented Dec 27, 2023

craterbot commented Dec 27, 2023

craterbot commented Dec 27, 2023

craterbot commented Dec 27, 2023

craterbot commented Dec 27, 2023

lcnr commented Jan 8, 2024

bors commented Jan 8, 2024

bors commented Jan 9, 2024

bors commented Jan 9, 2024

rust-timer commented Jan 9, 2024

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

aliemjay commented Dec 18, 2023 •

edited

Loading