-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. #122922

kpreid · 2024-03-23T01:06:27Z

This should assist comprehending the size of coroutines. In particular, whenever a future is suspended while awaiting another future, the latter is given the special name __awaitee, and now the type of the awaited future will be printed, allowing identifying caller/callee — er, I mean, poller/pollee — relationships.

It would be possible to include the type name in more cases, but I thought that that might be overly verbose (print-type-sizes is already a lot of text) and ordinary named fields or variables are easier for readers to discover the types of.

This change will also synergize with my other PR #122923 which changes type printing to print the path of the async fn instead of the span.

Implementation note: I'm not sure if Symbol::intern is appropriate for this application, but it was the obvious way to not have to remove the Copy implementation from FieldInfo, or add a 'tcx lifetime, while avoiding keeping a lot of possibly redundant strings in memory. I don't know what the proper tradeoff to make here is (though presumably it is not too important for a -Z debugging option).

rustbot · 2024-03-23T01:06:33Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

… locals. This should assist comprehending the size of coroutines. In particular, whenever a future is suspended while awaiting another future, the latter is given the special name `__awaitee`, and now the type of the awaited future will be printed, allowing identifying caller/callee — er, I mean, poller/pollee — relationships. It would be possible to include the type name in more cases, but I thought that that might be overly verbose (`print-type-sizes` is already a lot of text) and ordinary named fields or variables are easier for readers to discover the types of.

compiler-errors

Honestly don't see why we can't print this for all fields, tbh. Especially when types are monomorphized, even if you do know the variable it comes from, it's not always clear what the final types end up being.

compiler-errors · 2024-03-23T02:29:11Z

@bors r+

bors · 2024-03-23T02:29:13Z

📌 Commit 44d185b has been approved by compiler-errors

It is now in the queue for this repository.

compiler-errors · 2024-03-23T02:30:13Z

Actually wait, just put a Ty<'tcx> into FieldInfo. It implements Display.

@bors r-

compiler-errors · 2024-03-23T02:30:31Z

compiler/rustc_session/src/code_stats.rs

+ /// Name of the type of this field.
+ /// Present only if the creator thought that this would be important for identifying the field,
+ /// typically because the field name is uninformative.
+ pub type_name: Option<Symbol>,


Suggested change

pub type_name: Option<Symbol>,

pub type_name: Option<Ty<'tcx>>,

The Ty we have when constructing FieldInfo is a rustc_middle::ty::Ty. rustc_session, the crate containing FieldInfo, is a dependency of rustc_middle, so that would create a dependency cycle. What do you propose we do about that?

Oh lol

We could use a hook (compiler/rustc_middle/src/hooks/mod.rs) and pull the print_type_sizes + FieldInfo out of rustc_session and down into rustc_ty_utils?

That would have the added benefit of making the layout and the size printing code live together.

Well hm. CodeStats still lives in Session. I wonder if there's anywhere global this can live while also being mutable. Oh well, probably too much work.

global mutable state... 🙀

It's already global mutable state for the record lol

Fixing this would probably just be uplifting it from Session to GlobalCtxt

compiler-errors · 2024-03-23T02:58:33Z

Whatever that's too difficult

@bors r+

bors · 2024-03-23T02:58:35Z

📌 Commit 44d185b has been approved by compiler-errors

It is now in the queue for this repository.

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. This should assist comprehending the size of coroutines. In particular, whenever a future is suspended while awaiting another future, the latter is given the special name `__awaitee`, and now the type of the awaited future will be printed, allowing identifying caller/callee — er, I mean, poller/pollee — relationships. It would be possible to include the type name in more cases, but I thought that that might be overly verbose (`print-type-sizes` is already a lot of text) and ordinary named fields or variables are easier for readers to discover the types of. This change will also synergize with my other PR rust-lang#122923 which changes type printing to print the path of the `async fn` instead of the span. Implementation note: I'm not sure if `Symbol::intern` is appropriate for this application, but it was the obvious way to not have to remove the `Copy` implementation from `FieldInfo`, or add a `'tcx` lifetime, while avoiding keeping a lot of possibly redundant strings in memory. I don't know what the proper tradeoff to make here is (though presumably it is not too important for a `-Z` debugging option).

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#116016 (Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition) - rust-lang#122460 (Rework rmake support library API) - rust-lang#122658 (ci: Build gccjit from a git archive) - rust-lang#122698 (Cancel `cargo update` job if there's no updates) - rust-lang#122878 (Use `arch::wasm::unreachable` instead of `arch::wasm32::unreachable`) - rust-lang#122915 (Delay a bug if no RPITITs were found) - rust-lang#122916 (docs(sync): normalize dot in fn summaries) - rust-lang#122921 (Enable more mir-opt tests in debug builds) - rust-lang#122922 (-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals.) r? `@ghost` `@rustbot` modify labels: rollup

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. This should assist comprehending the size of coroutines. In particular, whenever a future is suspended while awaiting another future, the latter is given the special name `__awaitee`, and now the type of the awaited future will be printed, allowing identifying caller/callee — er, I mean, poller/pollee — relationships. It would be possible to include the type name in more cases, but I thought that that might be overly verbose (`print-type-sizes` is already a lot of text) and ordinary named fields or variables are easier for readers to discover the types of. This change will also synergize with my other PR rust-lang#122923 which changes type printing to print the path of the `async fn` instead of the span. Implementation note: I'm not sure if `Symbol::intern` is appropriate for this application, but it was the obvious way to not have to remove the `Copy` implementation from `FieldInfo`, or add a `'tcx` lifetime, while avoiding keeping a lot of possibly redundant strings in memory. I don't know what the proper tradeoff to make here is (though presumably it is not too important for a `-Z` debugging option).

…kingjubilee Rollup of 10 pull requests Successful merges: - rust-lang#121940 (Mention Register Size in `#[warn(asm_sub_register)]`) - rust-lang#122460 (Rework rmake support library API) - rust-lang#122698 (Cancel `cargo update` job if there's no updates) - rust-lang#122780 (Rename `hir::Local` into `hir::LetStmt`) - rust-lang#122875 (CFI: Support self_cell-like recursion) - rust-lang#122879 (CFI: Strip auto traits off Virtual calls) - rust-lang#122915 (Delay a bug if no RPITITs were found) - rust-lang#122916 (docs(sync): normalize dot in fn summaries) - rust-lang#122922 (-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals.) - rust-lang#122927 (Change an ICE regression test to use the original reproducer) r? `@ghost` `@rustbot` modify labels: rollup

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. This should assist comprehending the size of coroutines. In particular, whenever a future is suspended while awaiting another future, the latter is given the special name `__awaitee`, and now the type of the awaited future will be printed, allowing identifying caller/callee — er, I mean, poller/pollee — relationships. It would be possible to include the type name in more cases, but I thought that that might be overly verbose (`print-type-sizes` is already a lot of text) and ordinary named fields or variables are easier for readers to discover the types of. This change will also synergize with my other PR rust-lang#122923 which changes type printing to print the path of the `async fn` instead of the span. Implementation note: I'm not sure if `Symbol::intern` is appropriate for this application, but it was the obvious way to not have to remove the `Copy` implementation from `FieldInfo`, or add a `'tcx` lifetime, while avoiding keeping a lot of possibly redundant strings in memory. I don't know what the proper tradeoff to make here is (though presumably it is not too important for a `-Z` debugging option).

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#122460 (Rework rmake support library API) - rust-lang#122698 (Cancel `cargo update` job if there's no updates) - rust-lang#122780 (Rename `hir::Local` into `hir::LetStmt`) - rust-lang#122875 (CFI: Support self_cell-like recursion) - rust-lang#122915 (Delay a bug if no RPITITs were found) - rust-lang#122916 (docs(sync): normalize dot in fn summaries) - rust-lang#122921 (Enable more mir-opt tests in debug builds) - rust-lang#122922 (-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals.) - rust-lang#122927 (Change an ICE regression test to use the original reproducer) r? `@ghost` `@rustbot` modify labels: rollup

kpreid · 2024-03-23T14:11:16Z

Belated note to rollup creator @matthiaskrgr :

#122922 (this PR) and #122923 will conflict with each other in the merge queue — the former adds uses of the printing routine that the latter changes. So whichever one comes second will need re-blessing.

matthiaskrgr · 2024-03-23T14:26:46Z

uh oh, thanks!

…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#120577 (Stabilize slice_split_at_unchecked) - rust-lang#122698 (Cancel `cargo update` job if there's no updates) - rust-lang#122780 (Rename `hir::Local` into `hir::LetStmt`) - rust-lang#122915 (Delay a bug if no RPITITs were found) - rust-lang#122916 (docs(sync): normalize dot in fn summaries) - rust-lang#122921 (Enable more mir-opt tests in debug builds) - rust-lang#122922 (-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals.) - rust-lang#122927 (Change an ICE regression test to use the original reproducer) - rust-lang#122930 (add panic location to 'panicked while processing panic') - rust-lang#122931 (Fix some typos in the pin.rs) - rust-lang#122933 (tag_for_variant follow-ups) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#122922 - kpreid:print-async, r=compiler-errors -Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. This should assist comprehending the size of coroutines. In particular, whenever a future is suspended while awaiting another future, the latter is given the special name `__awaitee`, and now the type of the awaited future will be printed, allowing identifying caller/callee — er, I mean, poller/pollee — relationships. It would be possible to include the type name in more cases, but I thought that that might be overly verbose (`print-type-sizes` is already a lot of text) and ordinary named fields or variables are easier for readers to discover the types of. This change will also synergize with my other PR rust-lang#122923 which changes type printing to print the path of the `async fn` instead of the span. Implementation note: I'm not sure if `Symbol::intern` is appropriate for this application, but it was the obvious way to not have to remove the `Copy` implementation from `FieldInfo`, or add a `'tcx` lifetime, while avoiding keeping a lot of possibly redundant strings in memory. I don't know what the proper tradeoff to make here is (though presumably it is not too important for a `-Z` debugging option).

…-errors In `pretty_print_type()`, print `async fn` futures' paths instead of spans. This makes `-Zprint-type-sizes`'s output easier to read, because the name of an `async fn` is more immediately recognizable than its span. This change will also synergize with my other `-Zprint-type-sizes` PR rust-lang#122922 which prints the type of child futures being awaited. I also deleted the comment "FIXME(eddyb) should use `def_span`." because it appears to have already been fixed by commit 67727aa.

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. This should assist comprehending the size of coroutines. In particular, whenever a future is suspended while awaiting another future, the latter is given the special name `__awaitee`, and now the type of the awaited future will be printed, allowing identifying caller/callee — er, I mean, poller/pollee — relationships. It would be possible to include the type name in more cases, but I thought that that might be overly verbose (`print-type-sizes` is already a lot of text) and ordinary named fields or variables are easier for readers to discover the types of. This change will also synergize with my other PR rust-lang#122923 which changes type printing to print the path of the `async fn` instead of the span. Implementation note: I'm not sure if `Symbol::intern` is appropriate for this application, but it was the obvious way to not have to remove the `Copy` implementation from `FieldInfo`, or add a `'tcx` lifetime, while avoiding keeping a lot of possibly redundant strings in memory. I don't know what the proper tradeoff to make here is (though presumably it is not too important for a `-Z` debugging option).

…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#120577 (Stabilize slice_split_at_unchecked) - rust-lang#122698 (Cancel `cargo update` job if there's no updates) - rust-lang#122780 (Rename `hir::Local` into `hir::LetStmt`) - rust-lang#122915 (Delay a bug if no RPITITs were found) - rust-lang#122916 (docs(sync): normalize dot in fn summaries) - rust-lang#122921 (Enable more mir-opt tests in debug builds) - rust-lang#122922 (-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals.) - rust-lang#122927 (Change an ICE regression test to use the original reproducer) - rust-lang#122930 (add panic location to 'panicked while processing panic') - rust-lang#122931 (Fix some typos in the pin.rs) - rust-lang#122933 (tag_for_variant follow-ups) r? `@ghost` `@rustbot` modify labels: rollup

…-errors In `pretty_print_type()`, print `async fn` futures' paths instead of spans. This makes `-Zprint-type-sizes`'s output easier to read, because the name of an `async fn` is more immediately recognizable than its span. This change will also synergize with my other `-Zprint-type-sizes` PR rust-lang#122922 which prints the type of child futures being awaited. I also deleted the comment "FIXME(eddyb) should use `def_span`." because it appears to have already been fixed by commit 67727aa.

Rollup merge of rust-lang#122923 - kpreid:print-async-def, r=compiler-errors In `pretty_print_type()`, print `async fn` futures' paths instead of spans. This makes `-Zprint-type-sizes`'s output easier to read, because the name of an `async fn` is more immediately recognizable than its span. This change will also synergize with my other `-Zprint-type-sizes` PR rust-lang#122922 which prints the type of child futures being awaited. I also deleted the comment "FIXME(eddyb) should use `def_span`." because it appears to have already been fixed by commit 67727aa.

rustbot assigned fee1-dead Mar 23, 2024

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 Mar 23, 2024

kpreid force-pushed the print-async branch from 63b9322 to 44d185b Compare March 23, 2024 01:08

kpreid changed the title ~~-Zprint-type-sizes: print coroutine locals' types when they have no name.~~ -Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. Mar 23, 2024

kpreid mentioned this pull request Mar 23, 2024

In pretty_print_type(), print async fn futures' paths instead of spans. #122923

Merged

compiler-errors approved these changes Mar 23, 2024

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 Mar 23, 2024

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 Mar 23, 2024

compiler-errors reviewed Mar 23, 2024

View reviewed changes

compiler-errors assigned compiler-errors and unassigned fee1-dead Mar 23, 2024

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 Mar 23, 2024

matthiaskrgr mentioned this pull request Mar 23, 2024

Rollup of 9 pull requests #122925

Closed

workingjubilee mentioned this pull request Mar 23, 2024

Rollup of 10 pull requests #122929

Closed

matthiaskrgr mentioned this pull request Mar 23, 2024

Rollup of 9 pull requests #122932

Closed

matthiaskrgr mentioned this pull request Mar 23, 2024

Rollup of 11 pull requests #122947

Merged

bors merged commit 9418f69 into rust-lang:master Mar 23, 2024
11 checks passed

rustbot added this to the 1.79.0 milestone Mar 23, 2024

kpreid deleted the print-async branch March 23, 2024 19:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. #122922

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. #122922

kpreid commented Mar 23, 2024 •

edited

Loading

rustbot commented Mar 23, 2024

compiler-errors left a comment

compiler-errors commented Mar 23, 2024

bors commented Mar 23, 2024

compiler-errors commented Mar 23, 2024

compiler-errors Mar 23, 2024

kpreid Mar 23, 2024 •

edited

Loading

compiler-errors Mar 23, 2024

compiler-errors Mar 23, 2024

workingjubilee Mar 23, 2024

compiler-errors Mar 23, 2024

compiler-errors commented Mar 23, 2024

bors commented Mar 23, 2024

kpreid commented Mar 23, 2024

matthiaskrgr commented Mar 23, 2024

	pub type_name: Option<Symbol>,
	pub type_name: Option<Ty<'tcx>>,

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. #122922

-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals. #122922

Conversation

kpreid commented Mar 23, 2024 • edited Loading

rustbot commented Mar 23, 2024

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors commented Mar 23, 2024

bors commented Mar 23, 2024

compiler-errors commented Mar 23, 2024

compiler-errors Mar 23, 2024

Choose a reason for hiding this comment

kpreid Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

compiler-errors Mar 23, 2024

Choose a reason for hiding this comment

compiler-errors Mar 23, 2024

Choose a reason for hiding this comment

workingjubilee Mar 23, 2024

Choose a reason for hiding this comment

compiler-errors Mar 23, 2024

Choose a reason for hiding this comment

compiler-errors commented Mar 23, 2024

bors commented Mar 23, 2024

kpreid commented Mar 23, 2024

matthiaskrgr commented Mar 23, 2024

kpreid commented Mar 23, 2024 •

edited

Loading

kpreid Mar 23, 2024 •

edited

Loading