Add a missing `GenericTypeVisitable`, and avoid having interner traits for `FnSigKind` and `Abi` by ChayimFriedman2 · Pull Request #155562 · rust-lang/rust

ChayimFriedman2 · 2026-04-20T15:27:53Z

View all comments

r? types

rustbot · 2026-04-20T15:27:56Z

HIR ty lowering was modified

cc @fmease

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

rustbot · 2026-04-20T15:41:31Z

The Miri subtree was changed

cc @rust-lang/miri

nikomatsakis

Seems like a win to me.

If I'm following, @ChayimFriedman2, this basically gives you more shared code that can be reused?

View changes since this review

ChayimFriedman2 · 2026-04-27T19:39:25Z

Yes. As long as r-a and rustc don't need to diverge, we should prefer to share the code.

teor2345

Thank you, I was trying to work out how to do something like this in the original refactor, but couldn't quite get it to work.

Do you think there's much risk of a perf regression from #155223?

View changes since this review

ChayimFriedman2 · 2026-04-29T15:20:37Z

Do you think there's much risk of a perf regression from #155223?

How will this regress perf?

nikomatsakis · 2026-04-29T16:50:46Z

@bors delegate+

r=me once this is rebased etc.

rust-bors · 2026-04-29T16:50:49Z

✌️ @ChayimFriedman2, you can now approve this pull request!

If @nikomatsakis told you to "r=me" after making some further change, then please make that change and post @bors r=nikomatsakis.

View changes since this delegation.

Instead, just use the concrete types `FnSigKind` and `rustc_abi::ExternAbi`. This simplifies the code in both rustc and rust-analyzer.

rustbot · 2026-04-29T17:19:49Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

ChayimFriedman2 · 2026-04-29T17:20:07Z

@bors r=nikomatsakis

rust-bors · 2026-04-29T17:20:09Z

📌 Commit dff9d08 has been approved by nikomatsakis

It is now in the queue for this repository.

…atsakis Add a missing `GenericTypeVisitable`, and avoid having interner traits for `FnSigKind` and `Abi` r? types

…uwer Rollup of 22 pull requests Successful merges: - #154149 (resolve: Extend `ambiguous_import_visibilities` deprecation lint to glob-vs-glob ambiguities) - #155189 (simd_reduce_min/max: remove float support) - #155453 (apply Cortex-A53 errata 843419 mitigation to the AArch64 Linux targets) - #155562 (Add a missing `GenericTypeVisitable`, and avoid having interner traits for `FnSigKind` and `Abi`) - #155608 (rustc_middle: Implement the `partial_cmp` operation for `DefId`s) - #155721 (When archive format is wrong produce an error instead of ICE) - #155794 (privacy: share effective visibility initialization) - #155832 (c-variadic: more precise compatibility check in const-eval) - #155856 (std_detect: support detecting more features on aarch64 Windows) - #155861 (Suggest `[const] Trait` bounds in more places) - #155899 (`dlltool`: Set the working directory to workaround `--temp-prefix` bug) - #155916 (Update with new LLVM 22 target for `wasm32-wali-linux-musl` target) - #155935 (remap OUT_DIR paths to fix build script path leakage in crate metadata. ) - #155950 (use the new `//@ needs-asm-mnemonic: ret` more) - #155958 (ci(free-disk-space): remove more tools and fix warnings) - #155966 (miri subtree update) - #155711 (bump curl-sys and openssl-sys to support OpenSSL 4.0.x) - #155831 (Add `AcceptContext::expect_key_value`) - #155877 (Avoid misleading return-type note for foreign `Fn` callees) - #155949 (Update `opt_ast_lowering_delayed_lints` query to allow "stealing" lints, allowing to use `FnOnce` instead of `Fn`) - #155951 (Make `FlatMapInPlaceVec` an unsafe trait.) - #155967 (Fix `doc_cfg` feature for extern items)

…uwer Rollup of 21 pull requests Successful merges: - #155966 (miri subtree update) - #154149 (resolve: Extend `ambiguous_import_visibilities` deprecation lint to glob-vs-glob ambiguities) - #155189 (simd_reduce_min/max: remove float support) - #155562 (Add a missing `GenericTypeVisitable`, and avoid having interner traits for `FnSigKind` and `Abi`) - #155608 (rustc_middle: Implement the `partial_cmp` operation for `DefId`s) - #155721 (When archive format is wrong produce an error instead of ICE) - #155794 (privacy: share effective visibility initialization) - #155832 (c-variadic: more precise compatibility check in const-eval) - #155856 (std_detect: support detecting more features on aarch64 Windows) - #155861 (Suggest `[const] Trait` bounds in more places) - #155899 (`dlltool`: Set the working directory to workaround `--temp-prefix` bug) - #155916 (Update with new LLVM 22 target for `wasm32-wali-linux-musl` target) - #155935 (remap OUT_DIR paths to fix build script path leakage in crate metadata. ) - #155950 (use the new `//@ needs-asm-mnemonic: ret` more) - #155958 (ci(free-disk-space): remove more tools and fix warnings) - #155711 (bump curl-sys and openssl-sys to support OpenSSL 4.0.x) - #155831 (Add `AcceptContext::expect_key_value`) - #155877 (Avoid misleading return-type note for foreign `Fn` callees) - #155949 (Update `opt_ast_lowering_delayed_lints` query to allow "stealing" lints, allowing to use `FnOnce` instead of `Fn`) - #155951 (Make `FlatMapInPlaceVec` an unsafe trait.) - #155967 (Fix `doc_cfg` feature for extern items)

teor2345 · 2026-04-29T23:05:30Z

Do you think there's much risk of a perf regression from #155223?

How will this regress perf?

FnSig & FnSigKind are hot types, and some of their methods are called many times. My experience in #155223, #155852, and #153697 is that seemingly small changes can have a significant perf impact.

So far I've seen changing the size of the types and inlining improve perf. So I was asking to see if you knew of anything else that might have an impact.

If there's nothing, that's fine!
(And if there is an impact, we can probably recover perf using inlining.)

JonathanBrouwer · 2026-04-30T06:22:28Z

@rust-timer build 96e227a

…uwer Rollup of 21 pull requests Successful merges: - rust-lang/rust#155966 (miri subtree update) - rust-lang/rust#154149 (resolve: Extend `ambiguous_import_visibilities` deprecation lint to glob-vs-glob ambiguities) - rust-lang/rust#155189 (simd_reduce_min/max: remove float support) - rust-lang/rust#155562 (Add a missing `GenericTypeVisitable`, and avoid having interner traits for `FnSigKind` and `Abi`) - rust-lang/rust#155608 (rustc_middle: Implement the `partial_cmp` operation for `DefId`s) - rust-lang/rust#155721 (When archive format is wrong produce an error instead of ICE) - rust-lang/rust#155794 (privacy: share effective visibility initialization) - rust-lang/rust#155832 (c-variadic: more precise compatibility check in const-eval) - rust-lang/rust#155856 (std_detect: support detecting more features on aarch64 Windows) - rust-lang/rust#155861 (Suggest `[const] Trait` bounds in more places) - rust-lang/rust#155899 (`dlltool`: Set the working directory to workaround `--temp-prefix` bug) - rust-lang/rust#155916 (Update with new LLVM 22 target for `wasm32-wali-linux-musl` target) - rust-lang/rust#155935 (remap OUT_DIR paths to fix build script path leakage in crate metadata. ) - rust-lang/rust#155950 (use the new `//@ needs-asm-mnemonic: ret` more) - rust-lang/rust#155958 (ci(free-disk-space): remove more tools and fix warnings) - rust-lang/rust#155711 (bump curl-sys and openssl-sys to support OpenSSL 4.0.x) - rust-lang/rust#155831 (Add `AcceptContext::expect_key_value`) - rust-lang/rust#155877 (Avoid misleading return-type note for foreign `Fn` callees) - rust-lang/rust#155949 (Update `opt_ast_lowering_delayed_lints` query to allow "stealing" lints, allowing to use `FnOnce` instead of `Fn`) - rust-lang/rust#155951 (Make `FlatMapInPlaceVec` an unsafe trait.) - rust-lang/rust#155967 (Fix `doc_cfg` feature for extern items)

rust-timer · 2026-04-30T07:48:17Z

Finished benchmarking commit (96e227a): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

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

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.8%, secondary 1.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.8%	[2.8%, 2.8%]	1
Regressions ❌ (secondary)	1.6%	[1.6%, 1.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.8%	[2.8%, 2.8%]	1

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 487.214s -> 484.764s (-0.50%)
Artifact size: 390.94 MiB -> 390.94 MiB (0.00%)

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 20, 2026

rustbot assigned nikomatsakis Apr 20, 2026

This comment has been minimized.

Sign in to view

ChayimFriedman2 force-pushed the no-traits branch from 7ec1c80 to d3e8dd3 Compare April 20, 2026 15:41

This comment has been minimized.

Sign in to view

ChayimFriedman2 force-pushed the no-traits branch from d3e8dd3 to bd82b59 Compare April 21, 2026 19:54

This comment has been minimized.

Sign in to view

ChayimFriedman2 force-pushed the no-traits branch from 40a86de to 237818c Compare April 21, 2026 21:37

This comment has been minimized.

Sign in to view

ChayimFriedman2 force-pushed the no-traits branch from 237818c to 83b29b6 Compare April 22, 2026 01:27

oli-obk reviewed Apr 22, 2026

View reviewed changes

Comment thread compiler/rustc_type_ir/src/ty_kind.rs

nikomatsakis approved these changes Apr 27, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Impl GenericTypeVisitable for FnSigKind

3022538

teor2345 approved these changes Apr 29, 2026

View reviewed changes

Avoid having traits in rustc_type_ir for FnSigKind and Abi

dff9d08

Instead, just use the concrete types `FnSigKind` and `rustc_abi::ExternAbi`. This simplifies the code in both rustc and rust-analyzer.

ChayimFriedman2 force-pushed the no-traits branch from 83b29b6 to dff9d08 Compare April 29, 2026 17:19

rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 29, 2026

rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2026

JonathanBrouwer mentioned this pull request Apr 29, 2026

Rollup of 10 pull requests #155975

Closed

JonathanBrouwer mentioned this pull request Apr 29, 2026

Rollup of 22 pull requests #155976

Closed

JonathanBrouwer mentioned this pull request Apr 29, 2026

Rollup of 21 pull requests #155979

Merged

rust-bors Bot merged commit 5a40d36 into rust-lang:main Apr 30, 2026
11 checks passed

rustbot added this to the 1.97.0 milestone Apr 30, 2026

ChayimFriedman2 deleted the no-traits branch April 30, 2026 01:18

This comment has been minimized.

Sign in to view

Uh oh!

Conversation

ChayimFriedman2 commented Apr 20, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 20, 2026

Uh oh!

This comment has been minimized.

rustbot commented Apr 20, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

nikomatsakis left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChayimFriedman2 commented Apr 27, 2026

Uh oh!

This comment has been minimized.

teor2345 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChayimFriedman2 commented Apr 29, 2026

Uh oh!

nikomatsakis commented Apr 29, 2026

Uh oh!

rust-bors Bot commented Apr 29, 2026

Uh oh!

rustbot commented Apr 29, 2026

Uh oh!

ChayimFriedman2 commented Apr 29, 2026

Uh oh!

rust-bors Bot commented Apr 29, 2026

Uh oh!

teor2345 commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JonathanBrouwer commented Apr 30, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Apr 30, 2026

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ChayimFriedman2 commented Apr 20, 2026 •

edited by rustbot

Loading

nikomatsakis left a comment •

edited by rustbot

Loading

teor2345 left a comment •

edited by rustbot

Loading

teor2345 commented Apr 29, 2026 •

edited

Loading