c-variadic: add roundtrip test#155486
Conversation
rustbot
added
A-LLVM
|
|
This comment has been minimized.
This comment has been minimized.
| use std::ffi::*; | ||
|
|
||
| // Test that the rustc implementation of `va_arg` works with the LLVM implementation of passing | ||
| // c-variadic arguments. |
There was a problem hiding this comment.
I don't understand this comment. We don't use the LLVM implementation, we have our own, right?
There was a problem hiding this comment.
We have our own implementation of va_arg for the callee reading from a VaList. But how the caller passes the arguments, and how that VaList is constructed, is determined by LLVM.
There was a problem hiding this comment.
Ah :) We don't have any such tests yet...?!?
(Looking at the tests)
We do have a few things, but not covering all the types one can pass. Makes sense, thanks.
| // Intersperse a small type to test alignment logic. | ||
| assert!(ap.arg::<u32>() == 0xAAAA_AAAA); |
There was a problem hiding this comment.
4 bytes doesn't seem that small. Why not a 1-byte type?
There was a problem hiding this comment.
Because of the promotion rules/VaArgSafe, a c_int is the smallest thing we can read.
folkertdev
force-pushed
the
c-variadic-roundtrip
branch
from
77808a5 to
80ffa50
Compare
|
Some changes occurred in src/tools/cargo cc @ehuss |
|
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.
There was a problem hiding this comment.
something went wrong here
There was a problem hiding this comment.
yeah when you rebase but don't build git add -u does not play well with submodules... fixed now
folkertdev
force-pushed
the
c-variadic-roundtrip
branch
from
80ffa50 to
fa740c7
Compare
|
Looks good, thanks :) @bors r+ rollup |
rust-bors
bot
added
S-waiting-on-bors
…uwer Rollup of 12 pull requests Successful merges: - #155370 (Add regression test for dead code elimination with drop + panic) - #154823 (Replace the spdx-rs dependency with a minimal in-tree SPDX tag-value parser) - #155294 (Add test for coalescing of diagnostic attribute duplicates) - #155352 (triagebot.toml: Sync `assign.owners` with `autolabel."T-compiler"`) - #155431 (Add temporary scope to assert_matches) - #153873 (deprecate `std::char` constants and functions) - #154865 (libtest: use binary search for --exact test filtering) - #154979 (add #[must_use] macros for floats) - #155486 (c-variadic: add roundtrip test) - #155504 (Remove `AttributeLintKind` variants - part 2) - #155510 (Update Tidy python executable path) - #155514 (codegen-options docs: remove -Csoft-float)
Rollup merge of #155486 - folkertdev:c-variadic-roundtrip, r=RalfJung c-variadic: add roundtrip test tracking issue: #44930 Test that our `va_arg` implementation matches (as in, can decode) how LLVM passes c-variadic arguments. And some comment followup to #152980 (cc @RalfJung, feel free to review this PR too btw). r? tgross35
4 participants
tracking issue: #44930
Test that our
va_argimplementation matches (as in, can decode) how LLVM passes c-variadic arguments.And some comment followup to #152980 (cc @RalfJung, feel free to review this PR too btw).
r? tgross35