Do not mark BPF extern C returns as zeroext#158030
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dingxiangfei2009 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
|
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. |
This comment has been minimized.
This comment has been minimized.
7f7f697 to
5aaa542
Compare
5aaa542 to
00da20b
Compare
|
@bors r+ |
This comment has been minimized.
This comment has been minimized.
…bjorn3 Do not mark BPF `extern C` returns as `zeroext` Fixes #158028
This comment has been minimized.
This comment has been minimized.
|
💔 Test for d33895b failed: CI. Failed job:
|
|
It's unclear to me that there is a bug to fix here as the issue at hand rests on the question of what is the actual convention for 16-bit return values, as implemented by C compilers in practice? |
|
I looked at the clang source and it seemed like it didn't do any extension of return values. But then again, the code is not that easy to understand. |
|
@kevin-valerio When reporting issues about C interop that have bubbled up from a model, please verify it actually is a problem with gcc or clang if all the declarations are correct on both sides. If some code that originally, in C, is declared as having a signature like I don't think this patch is particularly awful, but you seem to be submitting quite a lot of things so I'd rather we not take them on good faith alone. |
|
Specifically, "This is definitely a problem, and a rustc developer doesn't have to speculatively read clang source code hoping to figure out what would actually be going on" makes it much easier to accept this (and also future patches where things might be more complicated and less clear that we should simply accept "playing defense" because the cost is admittedly probably low? ...but also I don't really know that it's that low because I know eBPF goes through a translation layer so things can cost more). |
Fixes #158028