Skip to content

Refactor FnDecl and FnSig non-type fields into a new wrapper type#155223

Open
teor2345 wants to merge 4 commits intorust-lang:mainfrom
teor2345:fndef-refactor
Open

Refactor FnDecl and FnSig non-type fields into a new wrapper type#155223
teor2345 wants to merge 4 commits intorust-lang:mainfrom
teor2345:fndef-refactor

Conversation

@teor2345
Copy link
Copy Markdown
Contributor

@teor2345 teor2345 commented Apr 13, 2026

Why this Refactor?

This PR is part of an initial cleanup for the arg splat experiment, but it's a useful refactor by itself.

It refactors the non-type fields of FnDecl, FnSig, and FnHeader into a new packed wrapper types, based on this comment in the splat experiment PR:
#153697 (comment)

It also refactors some common FnSig creation settings into their own methods. I did this instead of creating a struct with defaults.

Relationship to splat Experiment

I don't think we can use functional struct updates (..default()) to create FnDecl and FnSig, because we need the bit-packing for the splat experiment.

Bit-packing will avoid breaking "type is small" assertions for commonly used types when splat is added.
This PR packs these types:

  • ExternAbi: enum + unwind variants (38) -> 6 bits
  • ImplicitSelfKind: enum variants (5) -> 3 bits
  • lifetime_elision_allowed, safety, c_variadic: bool -> 1 bit

Minor Changes

Fixes some typos, and applies rustfmt to clippy files that got skipped somehow.

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 13, 2026
Comment thread compiler/rustc_codegen_cranelift/src/value_and_place.rs Outdated
@teor2345 teor2345 changed the title Refactor FnDef fields into a new wrapper type Refactor FnDecl and FnSig non-type fields into a new wrapper type Apr 15, 2026
@teor2345

This comment was marked as outdated.

@teor2345 teor2345 marked this pull request as ready for review April 15, 2026 21:15
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 15, 2026

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

HIR ty lowering was modified

cc @fmease

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

This PR changes rustc_public

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

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 15, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 15, 2026

r? @mati865

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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 72 candidates
  • Random selection from 17 candidates

Copy link
Copy Markdown
Contributor Author

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ready for review now, but I have some questions for reviewers (or compiler experts) about the legacy name mangling hash.

I'd also like to check if this PR impacts perf, either positively or negatively. I don't think I have permissions, but let's try anyway:

@bors try @rust-timer queue

View changes since this review

Comment thread tests/ui/symbol-names/basic.legacy.stderr
Comment thread compiler/rustc_codegen_cranelift/src/value_and_place.rs Outdated
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 15, 2026

@teor2345: 🔑 Insufficient privileges: not in try users

@nnethercote
Copy link
Copy Markdown
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Apr 16, 2026
Refactor FnDecl and FnSig non-type fields into a new wrapper type
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 16, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 16, 2026

☀️ Try build successful (CI)
Build commit: d647df9 (d647df90c918d4747135501dba51b294ffa85de3, parent: e8e4541ff19649d95afab52fdde2c2eaa6829965)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (d647df9): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.3% [-0.7%, -0.1%] 45
Improvements ✅
(secondary)
-0.6% [-1.4%, -0.2%] 68
All ❌✅ (primary) -0.3% [-0.7%, -0.1%] 45

Max RSS (memory usage)

Results (primary -1.8%, secondary 0.5%)

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

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.1% [0.9%, 5.3%] 2
Improvements ✅
(primary)
-1.8% [-1.8%, -1.8%] 1
Improvements ✅
(secondary)
-4.6% [-4.6%, -4.6%] 1
All ❌✅ (primary) -1.8% [-1.8%, -1.8%] 1

Cycles

Results (primary -2.0%, 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)
- - 0
Regressions ❌
(secondary)
8.0% [2.8%, 13.2%] 2
Improvements ✅
(primary)
-2.0% [-2.3%, -1.7%] 2
Improvements ✅
(secondary)
-3.2% [-4.3%, -2.1%] 12
All ❌✅ (primary) -2.0% [-2.3%, -1.7%] 2

Binary size

Results (primary -0.1%, secondary -0.2%)

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

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.1% [-0.2%, -0.0%] 63
Improvements ✅
(secondary)
-0.2% [-1.0%, -0.0%] 56
All ❌✅ (primary) -0.1% [-0.2%, -0.0%] 63

Bootstrap: 490.756s -> 487.632s (-0.64%)
Artifact size: 394.16 MiB -> 394.10 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 16, 2026
Comment thread compiler/rustc_abi/src/extern_abi.rs Outdated
$($e_name::$variant $( { unwind: $uw } )* => $tok,)*
}
}
// ALL_VARIANTS.iter().position(|v| v == self), but const
Copy link
Copy Markdown
Member

@RalfJung RalfJung Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

position returns an index, this here returns a bool -- something does not check out.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, that comment should have been on as_packed, it got left behind when I also needed a const equality check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants