compiler: fn ptrs should hit different lints based on ABI by workingjubilee · Pull Request #142271 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation6 Commits3 Checks9 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
compiler: fn ptrs should hint different lints based on ABI compiler: fn ptrs should hit different lints based on ABI
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.
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.
Co-authored-by: Ralf Jung post@ralfj.de
📌 Commit 8808a9c has been approved by RalfJung
It is now in the queue for this repository.
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
fmease added a commit to fmease/rust that referenced this pull request
…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
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::memchras#[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
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::memchras#[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
rust-timer added a commit that referenced this pull request
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
tgross35 pushed a commit to tgross35/compiler-builtins that referenced this pull request
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.