Do not call object_lifetime_default on lifetime params. #101214

cjgillot · 2022-08-30T21:14:09Z

Small cleanup to avoid unnecessary query invocations in trivial cases.

cjgillot · 2022-08-30T21:14:18Z

@bors try @rust-timer queue

rust-timer · 2022-08-30T21:14:19Z

Awaiting bors try build completion.

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

bors · 2022-08-30T21:14:25Z

⌛ Trying commit 93c6bea070d47b4878ef230860bd8ae1586c05d1 with merge 1b139018b0f4c6e673ea9aef642ae9f62f27f9a4...

bors · 2022-08-30T22:33:45Z

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

rust-timer · 2022-08-30T22:33:47Z

Queued 1b139018b0f4c6e673ea9aef642ae9f62f27f9a4 with parent 350cca3, future comparison URL.

rust-timer · 2022-08-31T00:27:30Z

Finished benchmarking commit (1b139018b0f4c6e673ea9aef642ae9f62f27f9a4): 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)	-1.2%	[-1.2%, -1.2%]	2
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)	2.1%	[2.1%, 2.1%]	1
Regressions ❌ (secondary)	4.8%	[4.8%, 4.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	2.1%	[2.1%, 2.1%]	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)	2.1%	[2.1%, 2.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.0%	[-4.9%, -3.1%]	2
All ❌✅ (primary)	-	-	0

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

cjgillot · 2022-08-31T21:39:53Z

r? compiler

estebank · 2022-09-01T16:09:56Z

compiler/rustc_passes/src/check_attr.rs

@@ -415,26 +413,21 @@ impl CheckAttrVisitor<'_> {
    }

    /// Debugging aid for `object_lifetime_default` query.
-    fn check_object_lifetime_default(&self, hir_id: HirId, span: Span) {
+    fn check_object_lifetime_default(&self, hir_id: HirId) {
        let tcx = self.tcx;
        if let Some(generics) = tcx.hir().get_generics(tcx.hir().local_def_id(hir_id)) {


This could be replaced with let/else { return; }, but isn't necessary in this PR.

estebank · 2022-09-01T16:12:55Z

compiler/rustc_resolve/src/late/lifetimes.rs

    let param_hir_id = tcx.local_def_id_to_hir_id(param_def_id);
-    let param = generics.params.iter().find(|p| p.hir_id == param_hir_id)?;
+    let param = generics.params.iter().find(|p| p.hir_id == param_hir_id).unwrap();

    // Scan the bounds and where-clauses on parameters to extract bounds
    // of the form `T:'a` so as to determine the `ObjectLifetimeDefault`
    // for each type parameter.
    match param.kind {


This can be if let now.

estebank · 2022-09-01T16:13:20Z

compiler/rustc_resolve/src/late/lifetimes.rs

-            // in an arbitrary order.
-            Some(ObjectLifetimeDefault::Empty)
+        _ => {
+            bug!("object_lifetime_default_raw must only be called on a type parameter")


Can we include some extra info on what this method was called on?

estebank · 2022-09-01T16:14:12Z

@bors r+

bors · 2022-09-01T16:14:14Z

📌 Commit ca287871a58918fe44245b9a4c51599bafdb9a28 has been approved by estebank

It is now in the queue for this repository.

bors · 2022-09-01T22:51:55Z

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

cjgillot · 2022-09-03T19:12:13Z

@bors r=estebank

bors · 2022-09-03T19:12:14Z

📌 Commit e716426 has been approved by estebank

It is now in the queue for this repository.

bors · 2022-09-03T19:32:44Z

⌛ Testing commit e716426 with merge 84f0c3f...

bors · 2022-09-03T21:56:13Z

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 84f0c3f to master...

rust-timer · 2022-09-03T23:11:37Z

Finished benchmarking commit (84f0c3f): 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)	1.6%	[1.6%, 1.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-0.8%, -0.6%]	5
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)	2.0%	[1.8%, 2.3%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.4%	[-5.4%, -3.3%]	2
All ❌✅ (primary)	-	-	0

Cycles

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

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

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

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

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 31, 2022

cjgillot force-pushed the old-no-lt branch from 93c6bea to ca28787 Compare August 31, 2022 19:23

rust-highfive assigned estebank Aug 31, 2022

cjgillot mentioned this pull request Aug 31, 2022

Remove separate indexing of early-bound regions #99821

Merged

estebank reviewed Sep 1, 2022

View reviewed changes

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 1, 2022

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 1, 2022

cjgillot force-pushed the old-no-lt branch from ca28787 to 26ac869 Compare September 2, 2022 16:24

Do not call object_lifetime_default on lifetime params.

e716426

cjgillot force-pushed the old-no-lt branch from 26ac869 to e716426 Compare September 3, 2022 19:11

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 3, 2022

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

bors merged commit 84f0c3f into rust-lang:master Sep 3, 2022

rustbot added this to the 1.65.0 milestone Sep 3, 2022

cjgillot deleted the old-no-lt branch September 4, 2022 08:57

fmease mentioned this pull request Aug 27, 2023

rustdoc: correctly deal with self ty params when eliding default object lifetimes #115276

Merged

matthiaskrgr mentioned this pull request Sep 3, 2023

ICE: has escaping bound vars, so it cannot be wrapped in a dummy binder. #115517

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not call object_lifetime_default on lifetime params. #101214

Do not call object_lifetime_default on lifetime params. #101214

cjgillot commented Aug 30, 2022 •

edited

Loading

cjgillot commented Aug 30, 2022

rust-timer commented Aug 30, 2022

bors commented Aug 30, 2022

This comment has been minimized.

bors commented Aug 30, 2022

rust-timer commented Aug 30, 2022

rust-timer commented Aug 31, 2022

cjgillot commented Aug 31, 2022

estebank Sep 1, 2022

estebank Sep 1, 2022

estebank Sep 1, 2022

estebank commented Sep 1, 2022

bors commented Sep 1, 2022

bors commented Sep 1, 2022

cjgillot commented Sep 3, 2022

bors commented Sep 3, 2022

bors commented Sep 3, 2022

bors commented Sep 3, 2022

rust-timer commented Sep 3, 2022

Do not call object_lifetime_default on lifetime params. #101214

Do not call object_lifetime_default on lifetime params. #101214

Conversation

cjgillot commented Aug 30, 2022 • edited Loading

cjgillot commented Aug 30, 2022

rust-timer commented Aug 30, 2022

bors commented Aug 30, 2022

This comment has been minimized.

bors commented Aug 30, 2022

rust-timer commented Aug 30, 2022

rust-timer commented Aug 31, 2022

Overall result: ✅ improvements - no action needed

Footnotes

cjgillot commented Aug 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estebank commented Sep 1, 2022

bors commented Sep 1, 2022

bors commented Sep 1, 2022

cjgillot commented Sep 3, 2022

bors commented Sep 3, 2022

bors commented Sep 3, 2022

bors commented Sep 3, 2022

rust-timer commented Sep 3, 2022

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

Footnotes

cjgillot commented Aug 30, 2022 •

edited

Loading