Make local query providers receive local keys #109092

compiler-errors · 2023-03-13T19:40:33Z

When a query is marked separate_provide_extern, we can map a query key to a "local" form of the key, e.g. DefId -> LocalDefId. This simplifies a ton of code which either has to assert or use something like expect_local to assert that the query key is local.

rustbot · 2023-03-13T19:40:40Z

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

compiler-errors · 2023-03-13T19:40:41Z

@bors try @rust-timer queue

rustbot · 2023-03-13T19:40:43Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occured in rustc_ty_utils::consts.rs

cc @BoxyUwU

bors · 2023-03-13T19:40:51Z

⌛ Trying commit 2a66c2b1ad8880024ad069b8c969102c23781708 with merge 0663769d33256fffe4d0e10d24119cb4cf2404be...

compiler/rustc_middle/src/query/keys.rs

Noratrieb

Very cool! One small improvement

compiler/rustc_middle/src/query/keys.rs

compiler-errors · 2023-03-13T22:23:14Z

@bors try @rust-timer queue

bors · 2023-03-13T22:23:23Z

⌛ Trying commit 9d2e1e07e89ebca3fe47fe9c5b742d789ff406ae with merge af096b934d3640666c1dcc82186802171479ffe0...

compiler-errors · 2023-03-13T23:34:14Z

@bors try

sorry bors lol

bors · 2023-03-13T23:34:26Z

⌛ Trying commit b3280e7dbaf7fb2950bc1fc51a6cd76e7c579e99 with merge ef8757fd3403378f1d31bbc8f9d428674b83ff13...

compiler/rustc_middle/src/ty/query.rs

bors · 2023-03-14T02:30:43Z

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

rust-timer · 2023-03-14T04:18:50Z

Finished benchmarking commit (ef8757fd3403378f1d31bbc8f9d428674b83ff13): 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.5%	[0.4%, 0.6%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-0.7%, -0.7%]	1
All ❌✅ (primary)	0.5%	[0.4%, 0.6%]	3

Max RSS (memory usage)

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

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
Improvements ✅ (secondary)	-5.0%	[-5.8%, -4.1%]	4
All ❌✅ (primary)	-	-	0

compiler-errors · 2023-03-14T04:21:02Z

Noise? 🤔

cjgillot

What a great cleanup!

compiler/rustc_middle/src/ty/query.rs

cjgillot · 2023-03-18T10:39:17Z

compiler/rustc_middle/src/query/keys.rs

@@ -45,14 +45,17 @@ pub trait Key: Sized {
    }
 }

+pub trait AsLocalKey: Key {


Why a separate trait instead of your initial implementation using the Key trait?

So that the as_local_key only has to be provided for the keys where we actually make use of it. This should make it a little easier to make sure that all of the impls are correct and not run into things like #109109 again.

Yeah, exactly what Nils said.

bors · 2023-03-21T15:22:58Z

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

cjgillot · 2023-03-21T16:44:11Z

@bors r+

bors · 2023-03-21T16:44:14Z

📌 Commit b1a957b has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-03-21T18:24:47Z

⌛ Testing commit b1a957b with merge 77d50a8...

bors · 2023-03-21T20:56:32Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 77d50a8 to master...

rust-timer · 2023-03-21T23:12:26Z

Finished benchmarking commit (77d50a8): 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)	-0.5%	[-0.6%, -0.4%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.5%	[-0.6%, -0.4%]	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)	1.1%	[1.1%, 1.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.7%	[-2.6%, -0.8%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.8%	[-2.6%, 1.1%]	3

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

Update the toolchain to use nightly-2023-04-16. Changes were related to the following changes to the toolchain: - rust-lang/rust#108944 - rust-lang/rust#108148 - rust-lang/rust#108471 - rust-lang/rust#109358 - rust-lang/rust#109849 - rust-lang/rust#109819 - rust-lang/rust#109718 - rust-lang/rust#109092 - rust-lang/rust#108856 - rust-lang/rust#105613 - rust-lang/rust#103042 - rust-lang/rust#109716 - rust-lang/rust#108340 - rust-lang/rust#102906 - rust-lang/rust#98112 - rust-lang/rust#108080

rustbot assigned cjgillot Mar 13, 2023

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 Mar 13, 2023

compiler-errors commented Mar 13, 2023

View reviewed changes

compiler/rustc_middle/src/query/keys.rs Outdated Show resolved Hide resolved

Noratrieb reviewed Mar 13, 2023

View reviewed changes

compiler/rustc_middle/src/query/keys.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Zoxc reviewed Mar 14, 2023

View reviewed changes

compiler/rustc_middle/src/ty/query.rs Outdated Show resolved Hide resolved

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 Mar 14, 2023

cjgillot reviewed Mar 18, 2023

View reviewed changes

cjgillot 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2023

compiler-errors added 6 commits March 21, 2023 15:38

Use local key in providers

2eb1c08

Use LocalDefId in ItemCtxt

979ef59

IdentitySubsts::identity_for_item takes Into<DefId>

7e65067

AsLocalKey trait

dcaf956

LocalCrate key

d213114

query_keys_local is conditional on separate_provide_if_extern

b1a957b

compiler-errors force-pushed the local-key branch from bc956fe to b1a957b Compare March 21, 2023 15:39

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 Mar 21, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 21, 2023

bors merged commit 77d50a8 into rust-lang:master Mar 21, 2023

rustbot added this to the 1.70.0 milestone Mar 21, 2023

rustbot removed the perf-regression Performance regression. label Mar 21, 2023

celinval mentioned this pull request Apr 26, 2023

Upgrade the toolchain to nightly-2023-04-16 model-checking/kani#2406

Merged

4 tasks

compiler-errors deleted the local-key branch August 11, 2023 20:06

matthiaskrgr mentioned this pull request Feb 13, 2024

ICE: downgrade_to_delayed_bug: cannot downgrade Warning to DelayedBug: not an error #121006

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make local query providers receive local keys #109092

Make local query providers receive local keys #109092

compiler-errors commented Mar 13, 2023

rustbot commented Mar 13, 2023

compiler-errors commented Mar 13, 2023

This comment has been minimized.

rustbot commented Mar 13, 2023

bors commented Mar 13, 2023

Noratrieb left a comment

compiler-errors commented Mar 13, 2023

This comment has been minimized.

bors commented Mar 13, 2023

compiler-errors commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 14, 2023

This comment has been minimized.

rust-timer commented Mar 14, 2023

compiler-errors commented Mar 14, 2023

cjgillot left a comment

cjgillot Mar 18, 2023

Noratrieb Mar 18, 2023

compiler-errors Mar 18, 2023

bors commented Mar 21, 2023

cjgillot commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 21, 2023

rust-timer commented Mar 21, 2023

Make local query providers receive local keys #109092

Make local query providers receive local keys #109092

Conversation

compiler-errors commented Mar 13, 2023

rustbot commented Mar 13, 2023

compiler-errors commented Mar 13, 2023

This comment has been minimized.

rustbot commented Mar 13, 2023

bors commented Mar 13, 2023

Noratrieb left a comment

Choose a reason for hiding this comment

compiler-errors commented Mar 13, 2023

This comment has been minimized.

bors commented Mar 13, 2023

compiler-errors commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 14, 2023

This comment has been minimized.

rust-timer commented Mar 14, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

compiler-errors commented Mar 14, 2023

cjgillot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Mar 21, 2023

cjgillot commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 21, 2023

bors commented Mar 21, 2023

rust-timer commented Mar 21, 2023

Overall result: ✅ improvements - no action needed