Skip to content

Make CodeStats' type_sizes public #139876

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 16, 2025
Merged

Conversation

blyxyas
Copy link
Member

@blyxyas blyxyas commented Apr 15, 2025

Add another way to get type sizes in CodeStats. I find it weird that the only way to get this information in block for all types is via printing directly to stdout. So this PR adds that flexibility.

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2025

r? @nnethercote

rustbot has assigned @nnethercote.
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

@rustbot 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 Apr 15, 2025
@compiler-errors
Copy link
Member

Could you explain a bit more what you need this for?

@blyxyas
Copy link
Member Author

blyxyas commented Apr 15, 2025

I'm making a Rust-specialized diff tool that not only gives us the raw diff, but also gives annotations on the core changes. One idea for an annotation I had was about changes in type sizes.

The tool could use -Zprint-type-sizes (or the internal function) and parse the output, or traverse the HIR in some way and execute layout_of on every Ty visible, but that would be more complex that it needs to be, and I can easily imagine other rustc-dependant tools or even the compiler itself using CodeStats.

@nnethercote
Copy link
Contributor

Would it suffice to make CodeStats::type_sizes public? That would be simpler and more flexible.

@blyxyas
Copy link
Member Author

blyxyas commented Apr 16, 2025

Absolutely, I didn't make it public to avoid CodeStats consumers manually fiddling with it and thus potential bugs, but in my specific use-case it's completely fine to just make it a public field. I'll change it

@blyxyas blyxyas changed the title Add get_type_sizes to CodeStats Make CodeStats' type_sizes public Apr 16, 2025
@nnethercote
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 16, 2025

📌 Commit 6999305 has been approved by nnethercote

It is now in the queue for this repository.

@bors 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 Apr 16, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 16, 2025
…rcote

Make CodeStats' type_sizes public

Add another way to get type sizes in CodeStats. I find it weird that the only way to get this information in block for all types is via printing directly to stdout. So this PR adds that flexibility.
@klensy
Copy link
Contributor

klensy commented Apr 16, 2025

Need some comment, otherwise this pub can be eventually removed as unused.
See for example #138511

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options)
 - rust-lang#139823 (Fix some bootstrap papercuts)
 - rust-lang#139853 (Disable combining LLD with external llvm-config)
 - rust-lang#139867 (Fix some tidy paper cuts)
 - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block)
 - rust-lang#139876 (Make CodeStats' type_sizes public)
 - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT)
 - rust-lang#139884 (Update books)
 - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks)
 - rust-lang#139893 (Add test for issue 125668)

r? `@ghost`
`@rustbot` modify labels: rollup
@blyxyas
Copy link
Member Author

blyxyas commented Apr 16, 2025

Is it too late for a comment, now that it's in rollup?

@nnethercote
Copy link
Contributor

Yeah, probably. You can just add the comment in a follow-up PR.

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options)
 - rust-lang#139823 (Fix some bootstrap papercuts)
 - rust-lang#139867 (Fix some tidy paper cuts)
 - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block)
 - rust-lang#139876 (Make CodeStats' type_sizes public)
 - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT)
 - rust-lang#139884 (Update books)
 - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks)
 - rust-lang#139893 (Add test for issue 125668)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#139647 (Add unstable parsing of `--extern foo::bar=libbar.rlib` command line options)
 - rust-lang#139823 (Fix some bootstrap papercuts)
 - rust-lang#139867 (Fix some tidy paper cuts)
 - rust-lang#139871 (Fix wrong "move keyword" suggestion for async gen block)
 - rust-lang#139876 (Make CodeStats' type_sizes public)
 - rust-lang#139880 (Don't compute name of associated item if it's an RPITIT)
 - rust-lang#139884 (Update books)
 - rust-lang#139886 (`borrowck_graphviz_*` attribute tweaks)
 - rust-lang#139893 (Add test for issue 125668)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b3284ad into rust-lang:master Apr 16, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 16, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2025
Rollup merge of rust-lang#139876 - blyxyas:write_type_sizes, r=nnethercote

Make CodeStats' type_sizes public

Add another way to get type sizes in CodeStats. I find it weird that the only way to get this information in block for all types is via printing directly to stdout. So this PR adds that flexibility.
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 21, 2025
…youxu

Document why CodeStats::type_sizes is public

As indicated in [this comment](rust-lang#139876 (comment)) from rust-lang#139876
> Need some comment, otherwise this pub can be eventually removed as unused.

r? `@nnethercote`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
Rollup merge of rust-lang#140121 - blyxyas:code_stats_pub_docs, r=jieyouxu

Document why CodeStats::type_sizes is public

As indicated in [this comment](rust-lang#139876 (comment)) from rust-lang#139876
> Need some comment, otherwise this pub can be eventually removed as unused.

r? `@nnethercote`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 22, 2025
Document why CodeStats::type_sizes is public

As indicated in [this comment](rust-lang/rust#139876 (comment)) from #139876
> Need some comment, otherwise this pub can be eventually removed as unused.

r? `@nnethercote`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants