Resolve lifetimes independently for each item-like. #103530

cjgillot · 2022-10-25T16:14:48Z

Now that the heavy-lifting is done on the AST and during lowering, we do not need to perform HIR lifetime resolution on a full item at once. Instead, we can treat each item-like independently, and look at generics_of the parent exceptionally for associated items.

rust-highfive · 2022-10-25T16:14:52Z

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

cjgillot · 2022-10-25T16:15:16Z

@bors try @rust-timer queue

rust-timer · 2022-10-25T16:15:17Z

Awaiting bors try build completion.

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

bors · 2022-10-25T16:15:25Z

⌛ Trying commit 4ad85429855d6f207f26be17875001be114b224a with merge 0640c07a0d4002c0a8944b68e5c83774614e5dbf...

bors · 2022-10-25T18:58:23Z

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

rust-timer · 2022-10-25T18:58:25Z

Queued 0640c07a0d4002c0a8944b68e5c83774614e5dbf with parent 85d089b, future comparison URL.

rust-timer · 2022-10-25T21:37:40Z

Finished benchmarking commit (0640c07a0d4002c0a8944b68e5c83774614e5dbf): 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-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
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-1.3%, -0.2%]	8
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)	8.9%	[0.9%, 20.3%]	25
Regressions ❌ (secondary)	3.9%	[3.7%, 4.1%]	2
Improvements ✅ (primary)	-5.4%	[-6.5%, -4.3%]	4
Improvements ✅ (secondary)	-4.9%	[-4.9%, -4.9%]	1
All ❌✅ (primary)	6.9%	[-6.5%, 20.3%]	29

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)	10.9%	[1.4%, 41.8%]	91
Regressions ❌ (secondary)	6.7%	[2.0%, 16.6%]	31
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	10.9%	[1.4%, 41.8%]	91

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

estebank · 2022-11-10T22:07:47Z

src/test/ui/impl-trait/nested-rpit-hrtb.stderr

+error: implementation of `Bar` is not general enough
+  --> $DIR/nested-rpit-hrtb.rs:51:93
+   |
+LL | fn one_hrtb_mention_fn_outlives_uses<'b>() -> impl for<'a> Bar<'a, Assoc = impl Sized + 'b> {}
+   |                                                                                             ^^ implementation of `Bar` is not general enough
+   |
+   = note: `()` must implement `Bar<'0>`, for any lifetime `'0`...
+   = note: ...but it actually implements `Bar<'1>`, for some specific lifetime `'1`


Can we do something to point at the return type here? Maybe special case functions that have no tail expression nor return statement?

src/test/ui/impl-trait/impl-fn-hrtb-bounds.stderr

src/test/ui/impl-trait/nested-rpit-hrtb.rs

compiler/rustc_hir_analysis/src/collect/lifetimes.rs

estebank

The code changes overall look fine, but I had a bunch of questions.

estebank · 2022-11-11T19:58:29Z

@bors r+

bors · 2022-11-11T19:58:31Z

📌 Commit 6c95805 has been approved by estebank

It is now in the queue for this repository.

bors · 2022-11-12T05:22:20Z

⌛ Testing commit 6c95805 with merge 825f8ed...

bors · 2022-11-12T08:02:55Z

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 825f8ed to master...

rust-timer · 2022-11-12T09:22:31Z

Finished benchmarking commit (825f8ed): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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.9%	[0.8%, 1.2%]	4
Regressions ❌ (secondary)	1.5%	[1.3%, 1.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.4%	[-3.4%, -3.4%]	1
All ❌✅ (primary)	0.9%	[0.8%, 1.2%]	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
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	-	-	0

…ebank Resolve lifetimes independently for each item-like. Now that the heavy-lifting is done on the AST and during lowering, we do not need to perform HIR lifetime resolution on a full item at once. Instead, we can treat each item-like independently, and look at `generics_of` the parent exceptionally for associated items.

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 25, 2022

rust-highfive assigned estebank Oct 25, 2022

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

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

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

cjgillot force-pushed the hir-lifetimes-direct branch from 4ad8542 to be610d0 Compare November 1, 2022 09:19

This comment has been minimized.

Sign in to view

cjgillot force-pushed the hir-lifetimes-direct branch from be610d0 to ad00b24 Compare November 1, 2022 11:35

estebank reviewed Nov 10, 2022

View reviewed changes

src/test/ui/impl-trait/impl-fn-hrtb-bounds.stderr Outdated Show resolved Hide resolved

estebank reviewed Nov 10, 2022

View reviewed changes

src/test/ui/impl-trait/nested-rpit-hrtb.rs Outdated Show resolved Hide resolved

estebank reviewed Nov 10, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/collect/lifetimes.rs Outdated Show resolved Hide resolved

estebank approved these changes Nov 10, 2022

View reviewed changes

estebank 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 Nov 10, 2022

cjgillot added 4 commits November 11, 2022 10:10

Tweak signatures in rustc_middle::hir::map.

0ff1d1e

Resolve lifetimes independently for each item-like.

44c10e4

Resolve lifetimes using the regular logic for RPIT.

3075f03

Visit opaque types during type collection too.

60892e8

cjgillot force-pushed the hir-lifetimes-direct branch from ad00b24 to 60892e8 Compare November 11, 2022 10:29

Clean-up formatting.

6c95805

cjgillot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 11, 2022

cjgillot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 11, 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 Nov 11, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 12, 2022

bors merged commit 825f8ed into rust-lang:master Nov 12, 2022

rustbot added this to the 1.67.0 milestone Nov 12, 2022

This was referenced Nov 12, 2022

Separate lifetime ident from lifetime resolution in HIR #104048

Merged

Support using Self or projections inside an RPIT/async fn #103491

Merged

cjgillot deleted the hir-lifetimes-direct branch November 12, 2022 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve lifetimes independently for each item-like. #103530

Resolve lifetimes independently for each item-like. #103530

cjgillot commented Oct 25, 2022

rust-highfive commented Oct 25, 2022

cjgillot commented Oct 25, 2022

rust-timer commented Oct 25, 2022

bors commented Oct 25, 2022

bors commented Oct 25, 2022

rust-timer commented Oct 25, 2022

rust-timer commented Oct 25, 2022

This comment has been minimized.

estebank Nov 10, 2022

estebank left a comment

estebank commented Nov 11, 2022

bors commented Nov 11, 2022

bors commented Nov 12, 2022

bors commented Nov 12, 2022

rust-timer commented Nov 12, 2022

Resolve lifetimes independently for each item-like. #103530

Resolve lifetimes independently for each item-like. #103530

Conversation

cjgillot commented Oct 25, 2022

rust-highfive commented Oct 25, 2022

cjgillot commented Oct 25, 2022

rust-timer commented Oct 25, 2022

bors commented Oct 25, 2022

bors commented Oct 25, 2022

rust-timer commented Oct 25, 2022

rust-timer commented Oct 25, 2022

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

This comment has been minimized.

estebank Nov 10, 2022

Choose a reason for hiding this comment

estebank left a comment

Choose a reason for hiding this comment

estebank commented Nov 11, 2022

bors commented Nov 11, 2022

bors commented Nov 12, 2022

bors commented Nov 12, 2022

rust-timer commented Nov 12, 2022

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles