Consolidate opaque ty and async fn lowering code #114487

compiler-errors · 2023-08-04T22:18:46Z

The codepaths for lowering "regular" opaques and async fn were almost identical, modulo some bookkeeping that seemed pretty easy to consolidate.

r? @cjgillot

compiler-errors · 2023-08-05T00:42:18Z

@bors try @rust-timer queue

bors · 2023-08-05T00:42:27Z

⌛ Trying commit fdb3cdd0f2733ecf495a96362ccf59dbf75401c7 with merge 5c8a43457e7206e930325fbb734b5858810086db...

bors · 2023-08-05T01:56:06Z

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

rust-timer · 2023-08-05T07:30:47Z

Finished benchmarking commit (5c8a43457e7206e930325fbb734b5858810086db): comparison URL.

Overall result: ❌ regressions - 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.7%	[0.2%, 1.5%]	39
Regressions ❌ (secondary)	0.7%	[0.4%, 1.0%]	9
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.2%, 1.5%]	39

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.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-1.7%	[-2.2%, -0.7%]	8
Improvements ✅ (secondary)	-3.0%	[-3.9%, -2.3%]	5
All ❌✅ (primary)	-1.7%	[-2.2%, -0.7%]	8

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)	2.0%	[1.7%, 2.1%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.0%	[1.7%, 2.1%]	3

Binary size

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

Bootstrap: 650.501s -> 650.065s (-0.07%)

cjgillot · 2023-08-05T07:28:26Z

compiler/rustc_ast_lowering/src/lib.rs

+            captured_lifetimes,
+            span,
+            opaque_ty_span,
+            |this| {
                // We have to be careful to get elision right here. The
                // idea is that we create a lifetime parameter for each
                // lifetime in the return type. So, given a return type


Should this comment be moved elsewhere?

I actually don't think that comment really makes sense anymore. I'm just gonna remove it.

compiler/rustc_ast_lowering/src/lib.rs

compiler-errors · 2023-08-05T16:47:11Z

I have no idea about that perf regression. Let's try again.

@bors try @rust-timer queue

bors · 2023-08-05T16:47:20Z

⌛ Trying commit bc39a888f2ef7455ca5979d6a25a406a0679d0e7 with merge be2985ab9700a63a51aae74e0d6c3f16ac980a17...

compiler-errors · 2023-08-05T16:53:44Z

@bors try

bors · 2023-08-05T16:53:53Z

⌛ Trying commit 169236e with merge ed7168ae0ecbc97500f8a1eea6e9ab5acc0c769b...

rust-log-analyzer · 2023-08-05T16:55:44Z

The job dist-x86_64-linux failed! Check out the build log: (web) (plain)

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

[TIMING] tool::ToolBuild { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu }, target: x86_64-unknown-linux-gnu, tool: "opt-dist", path: "src/tools/opt-dist", mode: ToolBootstrap, is_optional_tool: false, source_type: InTree, extra_features: [], allow_features: "" } -- 23.169
[TIMING] tool::OptimizedDist { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu }, target: x86_64-unknown-linux-gnu } -- 0.000
Build completed successfully in 0:00:23
+ ./build/x86_64-unknown-linux-gnu/stage0-tools-bin/opt-dist python3 ../x.py dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu --include-default-paths build-manifest bootstrap
[2023-08-05T16:50:34.252Z INFO  opt_dist] Skipping building of unimportant components for a try build
[2023-08-05T16:50:34.252Z INFO  opt_dist::utils::io] Resetting directory /tmp/tmp-multistage/opt-artifacts
[2023-08-05T16:50:34.252Z INFO  opt_dist::utils::io] Copying directory /tmp/rustc-perf to /tmp/tmp-multistage/opt-artifacts/rustc-perf
##[group]Environment values
Environment values
---
Caused by:
    Command RUST_BACKTRACE=full python3 /checkout/x.py build --target x86_64-unknown-linux-gnu --host x86_64-unknown-linux-gnu --stage 2 library/std --rust-profile-generate /tmp/tmp-multistage/opt-artifacts/rustc-pgo --set llvm.thin-lto=false --set llvm.link-shared=true [at /checkout/obj] has failed with exit code Some(1)

Stack backtrace:
   0: <opt_dist::exec::CmdBuilder>::run
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/exec.rs:78:17
   1: <opt_dist::exec::Bootstrap>::run
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/exec.rs:174:9
   2: opt_dist::execute_pipeline::{closure#1}::{closure#0}
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/main.rs:56:13
      <opt_dist::timer::TimerSection>::section::<opt_dist::execute_pipeline::{closure#1}::{closure#0}, ()>
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/timer.rs:111:22
      opt_dist::execute_pipeline::{closure#1}
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/main.rs:45:9
      <opt_dist::timer::TimerSection>::section::<opt_dist::execute_pipeline::{closure#1}, opt_dist::training::RustcPGOProfile>
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/timer.rs:111:22
   3: opt_dist::execute_pipeline
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/main.rs:42:29
             at /rustc/be2985ab9700a63a51aae74e0d6c3f16ac980a17/src/tools/opt-dist/src/main.rs:207:18
   4: <fn() -> core::result::Result<(), anyhow::Error> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/core/src/ops/function.rs:250:5
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/core/src/ops/function.rs:250:5
      std::sys_common::backtrace::__rust_begin_short_backtrace::<fn() -> core::result::Result<(), anyhow::Error>, core::result::Result<(), anyhow::Error>>
   5: std::rt::lang_start::<core::result::Result<(), anyhow::Error>>::{closure#0}
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/std/src/rt.rs:166:18
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/std/src/rt.rs:166:18
   6: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
      std::panicking::try::do_call
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/std/src/panicking.rs:500:40
      std::panicking::try
             at /rustc/7ba605cd90d6467bda469d1cd2148ac32ea562bf/library/std/src/panicking.rs:464:19

bors · 2023-08-05T18:06:58Z

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

rust-timer · 2023-08-06T02:36:52Z

Finished benchmarking commit (ed7168ae0ecbc97500f8a1eea6e9ab5acc0c769b): comparison URL.

Overall result: no relevant changes - 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 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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-3.2%, -3.2%]	1
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)	4.3%	[3.3%, 5.3%]	7
Regressions ❌ (secondary)	2.3%	[2.2%, 2.5%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.3%	[3.3%, 5.3%]	7

Binary size

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

Bootstrap: 648.941s -> 649.768s (0.13%)

cjgillot · 2023-08-06T09:39:40Z

@bors r+

bors · 2023-08-06T09:39:42Z

📌 Commit 169236e has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-08-06T11:16:34Z

⌛ Testing commit 169236e with merge bc720ad...

bors · 2023-08-06T13:04:30Z

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

rust-timer · 2023-08-06T14:31:47Z

Finished benchmarking commit (bc720ad): 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)	3.7%	[3.7%, 3.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.2%	[-1.2%, -1.2%]	1
All ❌✅ (primary)	3.7%	[3.7%, 3.7%]	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.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 651.351s -> 650.428s (-0.14%)

…vidtwco Record binder for bare trait object in LifetimeCollectVisitor The `LifetimeCollectVisitor` had a bug where it was not recording the binder of bate trait objects. This was uncovered in rust-lang#114487, when I changed opaque type lowering to ICE if it encountered a captured fresh lifetime with no def-id to map back to: https://github.com/rust-lang/rust/pull/114487/files#diff-ad0c15bbde97a607d4758ec7eaf88248be5d6b8ae084dfc84127f81e3f7a9bb4R1585 Fixes rust-lang#114664

rustbot assigned cjgillot Aug 4, 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 Aug 4, 2023

compiler-errors mentioned this pull request Aug 5, 2023

Make RPITITs capture all in-scope lifetimes #114489

Merged

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 Aug 5, 2023

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 Aug 5, 2023

cjgillot reviewed Aug 5, 2023

View reviewed changes

rust-cloud-vms bot force-pushed the opaques-refactoring-idk branch from fdb3cdd to bc39a88 Compare August 5, 2023 16:46

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 Aug 5, 2023

This comment has been minimized.

Sign in to view

compiler-errors added 2 commits August 5, 2023 16:53

Consolidate opaque ty and async fn lowering code

57a9689

a function is just another AnonymousCreateParameter rib

169236e

rust-cloud-vms bot force-pushed the opaques-refactoring-idk branch from bc39a88 to 169236e Compare August 5, 2023 16:53

This comment has been minimized.

Sign in to view

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Aug 6, 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 Aug 6, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 6, 2023

bors merged commit bc720ad into rust-lang:master Aug 6, 2023

rustbot added this to the 1.73.0 milestone Aug 6, 2023

compiler-errors deleted the opaques-refactoring-idk branch August 6, 2023 17:33

compiler-errors mentioned this pull request Aug 9, 2023

Record binder for bare trait object in LifetimeCollectVisitor #114667

Merged

matthiaskrgr mentioned this pull request Jun 23, 2024

ICE: no def-id for fresh lifetime #126850

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate opaque ty and async fn lowering code #114487

Consolidate opaque ty and async fn lowering code #114487

compiler-errors commented Aug 4, 2023

compiler-errors commented Aug 5, 2023

This comment has been minimized.

bors commented Aug 5, 2023

bors commented Aug 5, 2023

This comment has been minimized.

rust-timer commented Aug 5, 2023

cjgillot Aug 5, 2023

compiler-errors Aug 5, 2023

compiler-errors commented Aug 5, 2023

This comment has been minimized.

bors commented Aug 5, 2023

This comment has been minimized.

compiler-errors commented Aug 5, 2023

bors commented Aug 5, 2023

rust-log-analyzer commented Aug 5, 2023

bors commented Aug 5, 2023

This comment has been minimized.

rust-timer commented Aug 6, 2023

cjgillot commented Aug 6, 2023

bors commented Aug 6, 2023

bors commented Aug 6, 2023

bors commented Aug 6, 2023

rust-timer commented Aug 6, 2023

Consolidate opaque ty and async fn lowering code #114487

Consolidate opaque ty and async fn lowering code #114487

Conversation

compiler-errors commented Aug 4, 2023

compiler-errors commented Aug 5, 2023

This comment has been minimized.

bors commented Aug 5, 2023

bors commented Aug 5, 2023

This comment has been minimized.

rust-timer commented Aug 5, 2023

Overall result: ❌ regressions - ACTION NEEDED

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Aug 5, 2023

This comment has been minimized.

bors commented Aug 5, 2023

This comment has been minimized.

compiler-errors commented Aug 5, 2023

bors commented Aug 5, 2023

rust-log-analyzer commented Aug 5, 2023

bors commented Aug 5, 2023

This comment has been minimized.

rust-timer commented Aug 6, 2023

Overall result: no relevant changes - no action needed

cjgillot commented Aug 6, 2023

bors commented Aug 6, 2023

bors commented Aug 6, 2023

bors commented Aug 6, 2023

rust-timer commented Aug 6, 2023

Overall result: no relevant changes - no action needed