Separate definitions and HIR owners in the type system #102040

TaKO8Ki · 2022-09-20T06:12:57Z

Fixes #83158

r? @cjgillot

rustbot · 2022-09-20T06:13:02Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

TaKO8Ki · 2022-09-20T09:16:32Z

@bors try @rust-timer queue

rust-timer · 2022-09-20T09:16:33Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-09-20T09:16:44Z

⌛ Trying commit 9ef994ecf1e229cfe0bc915893e872779822e484 with merge fc0ede293f9529a80d346d21cf20f5f8cb8b6b8d...

bors · 2022-09-20T10:43:37Z

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

rust-timer · 2022-09-20T10:43:39Z

Queued fc0ede293f9529a80d346d21cf20f5f8cb8b6b8d with parent acb8934, future comparison URL.

rust-timer · 2022-09-20T13:22:10Z

Finished benchmarking commit (fc0ede293f9529a80d346d21cf20f5f8cb8b6b8d): comparison URL.

Overall result: ❌ regressions - 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-review -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.3%	[0.2%, 0.4%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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)	5.0%	[2.7%, 7.2%]	2
Improvements ✅ (primary)	-3.0%	[-3.0%, -3.0%]	1
Improvements ✅ (secondary)	-5.4%	[-5.4%, -5.4%]	1
All ❌✅ (primary)	-3.0%	[-3.0%, -3.0%]	1

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

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

TaKO8Ki · 2022-09-20T13:37:33Z

@bors try @rust-timer queue

rust-timer · 2022-09-20T13:37:36Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-09-20T13:37:50Z

⌛ Trying commit a61e3632316cb4a90370ca420cac7486efeeb414 with merge ec9258ea1ae0965d54d386a2c3c7e90f70fc8494...

bors · 2022-09-20T15:28:13Z

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

rust-timer · 2022-09-20T15:28:16Z

Queued ec9258ea1ae0965d54d386a2c3c7e90f70fc8494 with parent 8fd6d03, future comparison URL.

rust-timer · 2022-09-20T16:45:31Z

Finished benchmarking commit (ec9258ea1ae0965d54d386a2c3c7e90f70fc8494): comparison URL.

Overall result: ❌ regressions - 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-review -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.3%	[0.2%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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)	8.1%	[3.2%, 12.9%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.7%	[-3.2%, -2.2%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.7%	[-3.2%, 12.9%]	4

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

camsteffen · 2022-09-20T18:16:08Z

compiler/rustc_hir/src/hir.rs

@@ -2206,14 +2206,14 @@ pub struct FnSig<'hir> {
 // so it can fetched later.
 #[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Debug, HashStable_Generic)]
 pub struct TraitItemId {
-    pub def_id: LocalDefId,
+    pub def_id: OwnerId,


I think any OwnerId fields or variables should not be named def_id.

cjgillot · 2022-09-20T18:22:47Z

compiler/rustc_borrowck/src/diagnostics/region_name.rs

@@ -302,7 +302,9 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
                    if free_region.bound_region.is_named() {
                        // A named region that is actually named.
                        Some(RegionName { name, source: RegionNameSource::NamedFreeRegion(span) })
-                    } else if let hir::IsAsync::Async = tcx.asyncness(self.mir_hir_id().owner) {
+                    } else if let hir::IsAsync::Async =
+                        tcx.asyncness(self.mir_hir_id().owner.def_id)


Pre-existing: why mir_hir_id().owner and not mir_def_id()?

compiler/rustc_hir/src/hir_id.rs

compiler/rustc_lint/src/builtin.rs

cjgillot · 2022-09-20T18:30:56Z

compiler/rustc_middle/src/query/mod.rs

@@ -1588,7 +1588,7 @@ rustc_queries! {
        arena_cache
        desc { "resolving lifetimes" }
    }
-    query named_region_map(_: LocalDefId) ->
+    query named_region_map(_: hir::OwnerId) ->


This requires to add an impl DepNodeParams for OwnerId which copies the one for LocalDefId.

lqd · 2022-09-21T10:57:03Z

It would be cool to add some documentation here: in comments in the code and maybe the HIR chapter in the rustc dev guide. The body of #83158 can be used as a base for OwnerId at least. What do you think ?

fix a ui test use `into` fix clippy ui test fix a run-make-fulldeps test implement `IntoQueryParam<DefId>` for `OwnerId` use `OwnerId` for more queries change the type of `ParentOwnerIterator::Item` to `(OwnerId, OwnerNode)`

TaKO8Ki · 2022-09-24T14:53:29Z

@bors r=cjgillot

bors · 2022-09-24T14:53:30Z

📌 Commit 8fe9360 has been approved by cjgillot

It is now in the queue for this repository.

bors · 2022-09-24T17:17:45Z

⌛ Testing commit 8fe9360 with merge c61dde4cc4662710d7acef61383403d04a0a5d31...

bors · 2022-09-24T17:19:15Z

💔 Test failed - checks-actions

TaKO8Ki · 2022-09-24T17:22:58Z

@bors r=cjgillot

bors · 2022-09-24T17:23:00Z

💡 This pull request was already approved, no need to approve it again.

This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
There's another pull request that is currently being tested, blocking this pull request: [beta] Bump stage0 #102179

bors · 2022-09-24T17:23:00Z

📌 Commit 8fe9360 has been approved by cjgillot

It is now in the queue for this repository.

TaKO8Ki · 2022-09-24T17:23:23Z

@bors retry

bors · 2022-09-24T22:42:10Z

⌛ Testing commit 8fe9360 with merge d0ece44...

bors · 2022-09-25T00:57:45Z

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

rust-timer · 2022-09-25T02:02:59Z

Finished benchmarking commit (d0ece44): comparison URL.

Overall result: ❌ regressions - 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)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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.7%	[-3.3%, -2.2%]	2
All ❌✅ (primary)	-	-	0

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)	3.1%	[2.5%, 3.6%]	2
Regressions ❌ (secondary)	2.6%	[2.0%, 3.2%]	2
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.2%	[-2.7%, 3.6%]	3

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

rust-log-analyzer · 2022-09-25T10:00:49Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

…tead of `LocalDefId` <rust-lang/rust#102040>

rust-highfive assigned cjgillot Sep 20, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2022

TaKO8Ki force-pushed the separate-definitions-and-hir-owners branch from c8f5d30 to c6f5ed6 Compare September 20, 2022 06:22

TaKO8Ki changed the title ~~Separate definitions and HIR owners~~ Separate definitions and HIR owners in the type system Sep 20, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 20, 2022

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 Sep 20, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 20, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 20, 2022

camsteffen reviewed Sep 20, 2022

View reviewed changes

cjgillot reviewed Sep 20, 2022

View reviewed changes

separate definitions and HIR owners

8fe9360

fix a ui test use `into` fix clippy ui test fix a run-make-fulldeps test implement `IntoQueryParam<DefId>` for `OwnerId` use `OwnerId` for more queries change the type of `ParentOwnerIterator::Item` to `(OwnerId, OwnerNode)`

TaKO8Ki force-pushed the separate-definitions-and-hir-owners branch from fcc8399 to 8fe9360 Compare September 24, 2022 14:22

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 Sep 24, 2022

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 24, 2022

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 Sep 24, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 25, 2022

bors merged commit d0ece44 into rust-lang:master Sep 25, 2022

rustbot added this to the 1.66.0 milestone Sep 25, 2022

bors mentioned this pull request Sep 25, 2022

Make traits_in_crate and impls_in_crate proper queries #100601

Closed

4 tasks

TaKO8Ki deleted the separate-definitions-and-hir-owners branch September 25, 2022 01:45

bors mentioned this pull request Sep 25, 2022

Compute lint levels by definition #102236

Merged

yvt added a commit to yvt/servo that referenced this pull request Oct 16, 2022

fix(script_plugins): Map::get_parent_item now returns OwnerId ins…

8a0f777

…tead of `LocalDefId` <rust-lang/rust#102040>

yvt added a commit to yvt/servo that referenced this pull request Oct 16, 2022

fix(script_plugins): Map::get_parent_item now returns OwnerId ins…

00f6b6f

…tead of `LocalDefId` <rust-lang/rust#102040>

compiler-errors mentioned this pull request Jun 21, 2023

Rust allows impl Fn(T<'a>) -> T<'b> to be : 'static, which is unsound #112905

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate definitions and HIR owners in the type system #102040

Separate definitions and HIR owners in the type system #102040

TaKO8Ki commented Sep 20, 2022

rustbot commented Sep 20, 2022

TaKO8Ki commented Sep 20, 2022

rust-timer commented Sep 20, 2022

bors commented Sep 20, 2022

bors commented Sep 20, 2022

rust-timer commented Sep 20, 2022

This comment has been minimized.

rust-timer commented Sep 20, 2022

TaKO8Ki commented Sep 20, 2022

rust-timer commented Sep 20, 2022

bors commented Sep 20, 2022

bors commented Sep 20, 2022

rust-timer commented Sep 20, 2022

rust-timer commented Sep 20, 2022

camsteffen Sep 20, 2022

cjgillot Sep 20, 2022

cjgillot Sep 20, 2022

lqd commented Sep 21, 2022

TaKO8Ki commented Sep 24, 2022

bors commented Sep 24, 2022

bors commented Sep 24, 2022

bors commented Sep 24, 2022

TaKO8Ki commented Sep 24, 2022

bors commented Sep 24, 2022

bors commented Sep 24, 2022

TaKO8Ki commented Sep 24, 2022 •

edited

Loading

bors commented Sep 24, 2022

bors commented Sep 25, 2022

rust-timer commented Sep 25, 2022

rust-log-analyzer commented Sep 25, 2022

Separate definitions and HIR owners in the type system #102040

Separate definitions and HIR owners in the type system #102040

Conversation

TaKO8Ki commented Sep 20, 2022

rustbot commented Sep 20, 2022

TaKO8Ki commented Sep 20, 2022

rust-timer commented Sep 20, 2022

bors commented Sep 20, 2022

bors commented Sep 20, 2022

rust-timer commented Sep 20, 2022

This comment has been minimized.

rust-timer commented Sep 20, 2022

Overall result: ❌ regressions - no action needed

Footnotes

TaKO8Ki commented Sep 20, 2022

rust-timer commented Sep 20, 2022

bors commented Sep 20, 2022

bors commented Sep 20, 2022

rust-timer commented Sep 20, 2022

rust-timer commented Sep 20, 2022

Overall result: ❌ regressions - no action needed

Footnotes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lqd commented Sep 21, 2022

TaKO8Ki commented Sep 24, 2022

bors commented Sep 24, 2022

bors commented Sep 24, 2022

bors commented Sep 24, 2022

TaKO8Ki commented Sep 24, 2022

bors commented Sep 24, 2022

bors commented Sep 24, 2022

TaKO8Ki commented Sep 24, 2022 • edited Loading

bors commented Sep 24, 2022

bors commented Sep 25, 2022

rust-timer commented Sep 25, 2022

Overall result: ❌ regressions - no action needed

Footnotes

rust-log-analyzer commented Sep 25, 2022

TaKO8Ki commented Sep 24, 2022 •

edited

Loading