Skip to content

Conversation

folkertdev
Copy link
Contributor

tracking issue: #44930

A modification of #146454, keeping just the documentation changes, but not unsealing the trait.

Although conceptually we'd want to unseal the trait, there are many edge cases to supporting arbitrary types. We'd need to exhaustively test that all targets/calling conventions support all types that rust might generate (or generate proper error messages for unsupported cases). At present, many of the va_arg implementations assume that the argument is a scalar, and has an alignment of at most 8. That is totally sufficient for an MVP (accepting all of the "standard" C types), but clearly does not cover all rust types.

This PR also adds some various other tests for edge cases of c-variadic:

  • the #[inline] attribute in its various forms. At present, LLVM is unable to inline c-variadic functions, but the attribute should still be accepted. #[rustc_force_inline] already rejects c-variadic functions.
  • naked functions should accept and work with a C variable argument list. In the future we'd like to allow more ABIs with naked functions (basically, any ABI for which we accept defining foreign c-variadic functions), but for now only "C" and "C-unwind are supported
  • guaranteed tail calls: c-variadic functions cannot be tail-called. That was already rejected, but there was not test for it.

r? @workingjubilee

and document `VaList::arg`.
as far as I can see this was not tested, though the error message was already implemented
…unctions

they don't do anything, because LLVM is unable to inline c-variadic functions (on most targets, anyway)
@rustbot rustbot added the F-explicit_tail_calls `#![feature(explicit_tail_calls)]` label Sep 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2025

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 13, 2025
@folkertdev folkertdev added the F-c_variadic `#![feature(c_variadic)]` label Sep 13, 2025
@workingjubilee
Copy link
Member

Thanks!

I think I will go ahead and shove this into the queue since this is an already discussed, docs-and-tests-only change.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 13, 2025

📌 Commit a107ea1 has been approved by workingjubilee

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 Sep 13, 2025
bors added a commit that referenced this pull request Sep 13, 2025
Rollup of 8 pull requests

Successful merges:

 - #113095 (Document `become` keyword)
 - #146159 (Some hygiene doc improvements)
 - #146171 (tidy: check that error messages don't start with a capitalized letter)
 - #146419 (Update the arm-* and aarch64-* platform docs.)
 - #146473 (Revert "Constify SystemTime methods")
 - #146506 (Fix small typo in check-cfg.md)
 - #146517 (fix Condvar::wait_timeout docs)
 - #146521 (document `core::ffi::VaArgSafe`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit da1c27d into rust-lang:master Sep 14, 2025
10 checks passed
rust-timer added a commit that referenced this pull request Sep 14, 2025
Rollup merge of #146521 - folkertdev:document-va-arg-safe, r=workingjubilee

document `core::ffi::VaArgSafe`

tracking issue: #44930

A modification of #146454, keeping just the documentation changes, but not unsealing the trait.

Although conceptually we'd want to unseal the trait, there are many edge cases to supporting arbitrary types. We'd need to exhaustively test that all targets/calling conventions support all types that rust might generate (or generate proper error messages for unsupported cases). At present, many of the `va_arg` implementations assume that the argument is a scalar, and has an alignment of at most 8. That is totally  sufficient for an MVP (accepting all of the "standard" C types), but clearly does not cover all rust types.

This PR also adds some various other tests for edge cases of c-variadic:

- the `#[inline]` attribute in its various forms. At present, LLVM is unable to inline c-variadic functions, but the attribute should still be accepted. `#[rustc_force_inline]` already rejects c-variadic functions.
- naked functions should accept and work with a C variable argument list. In the future we'd like to allow more ABIs with naked functions (basically, any ABI for which we accept defining foreign c-variadic functions), but for now only  `"C"` and `"C-unwind` are supported
- guaranteed tail calls: c-variadic functions cannot be tail-called. That was already rejected, but there was not test for it.

r? `@workingjubilee`
@rustbot rustbot added this to the 1.91.0 milestone Sep 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-c_variadic `#![feature(c_variadic)]` F-explicit_tail_calls `#![feature(explicit_tail_calls)]` 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. T-libs Relevant to the library 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