Skip to content

compiler: fn ptrs should hit different lints based on ABI #142271

New issue

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

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

Already on GitHub? Sign in to your account

Merged

Conversation

workingjubilee
Copy link
Member

I was looking closer at the code for linting on ABIs and realized a mistake was probably made during rebase or review. I think that for function pointers in the HIR, the lint that fires should probably depend on the ABI we encountered, e.g. if it's on the newly-deprecated set of ABIs or not. This will be slightly confusing for a little bit, but I think we can do more to reduce that confusion by switching unsupported_fn_ptr_calling_conventions to a hard error.

r? @RalfJung

@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 Jun 9, 2025
@workingjubilee workingjubilee changed the title compiler: fn ptrs should hint different lints based on ABI compiler: fn ptrs should hit different lints based on ABI Jun 9, 2025
@RalfJung
Copy link
Member

From my side this was deliberate: UNSUPPORTED_FN_PTR_CALLING_CONVENTIONS now also triggers for these newly deprecated calling conventions. I didn't think it'd be worth it to think much about triggering one lint vs the other here.

@RalfJung
Copy link
Member

Ah, is this in preparation for #142134, i.e. to make UNSUPPORTED_FN_PTR_CALLING_CONVENTIONS a hard error Soon (TM)? In that case, yeah this LGTM. r=me with comment nit fixed.

@workingjubilee
Copy link
Member Author

Yes, exactly!

@workingjubilee
Copy link
Member Author

@bors r=RalfJung

@bors
Copy link
Collaborator

bors commented Jun 10, 2025

📌 Commit 8808a9c has been approved by RalfJung

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 Jun 10, 2025
fmease added a commit to fmease/rust that referenced this pull request Jun 10, 2025
…fferent-lints, r=RalfJung

compiler: fn ptrs should hit different lints based on ABI

I was looking closer at the code for linting on ABIs and realized a mistake was probably made during rebase or review. I think that for function pointers in the HIR, the lint that fires should probably depend on the ABI we encountered, e.g. if it's on the newly-deprecated set of ABIs or not. This will be slightly confusing for a little bit, but I think we can do more to reduce that confusion by switching `unsupported_fn_ptr_calling_conventions` to a hard error.

r? `@RalfJung`
bors added a commit that referenced this pull request Jun 10, 2025
Rollup of 14 pull requests

Successful merges:

 - #134442 (Specify the behavior of `file!`)
 - #134841 (Look at proc-macro attributes when encountering unknown attribute)
 - #140372 (Exhaustively handle parsed attributes in CheckAttr)
 - #140766 (Stabilize keylocker)
 - #141642 (Note the version and PR of removed features when using it)
 - #141909 (Add central execution context to bootstrap)
 - #141992 (use `#[naked]` for `__rust_probestack`)
 - #142102 (docs: Small clarification on the usage of read_to_string and read_to_end trait methods)
 - #142124 (Allow transmute casts in pre-runtime-MIR)
 - #142240 (deduplicate the rest of AST walker functions)
 - #142258 (platform-support.md: Mention specific Linux kernel version or later)
 - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`)
 - #142271 (compiler: fn ptrs should hit different lints based on ABI)
 - #142288 (const_eval: fix some outdated comments)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jun 10, 2025
Rollup of 16 pull requests

Successful merges:

 - #134442 (Specify the behavior of `file!`)
 - #140372 (Exhaustively handle parsed attributes in CheckAttr)
 - #140766 (Stabilize keylocker)
 - #141642 (Note the version and PR of removed features when using it)
 - #141818 (Don't create .msi installer for gnullvm hosts)
 - #141909 (Add central execution context to bootstrap)
 - #141992 (use `#[naked]` for `__rust_probestack`)
 - #142101 (core::ptr: deduplicate more method docs)
 - #142102 (docs: Small clarification on the usage of read_to_string and read_to_end trait methods)
 - #142124 (Allow transmute casts in pre-runtime-MIR)
 - #142240 (deduplicate the rest of AST walker functions)
 - #142258 (platform-support.md: Mention specific Linux kernel version or later)
 - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`)
 - #142271 (compiler: fn ptrs should hit different lints based on ABI)
 - #142275 (rustdoc: Refractor `clean_ty_generics`)
 - #142288 (const_eval: fix some outdated comments)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a2badeb into rust-lang:master Jun 10, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 10, 2025
rust-timer added a commit that referenced this pull request Jun 10, 2025
Rollup merge of #142271 - workingjubilee:fn-ptrs-have-two-different-lints, r=RalfJung

compiler: fn ptrs should hit different lints based on ABI

I was looking closer at the code for linting on ABIs and realized a mistake was probably made during rebase or review. I think that for function pointers in the HIR, the lint that fires should probably depend on the ABI we encountered, e.g. if it's on the newly-deprecated set of ABIs or not. This will be slightly confusing for a little bit, but I think we can do more to reduce that confusion by switching `unsupported_fn_ptr_calling_conventions` to a hard error.

r? ``@RalfJung``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

4 participants