c-variadic: unseal core::ffi::VaArgSafe
#146454
Conversation
rustbot
added
S-waiting-on-review
|
|
folkertdev
force-pushed
the
unseal-va-arg-safe
branch
from
1951de5 to
36b9646
Compare
|
|
This comment has been minimized.
This comment has been minimized.
|
Based no our reading of https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf#page=302, something like If a complex type contains fields of types that are not supported by the ABI, then it's not guaranteed that it can be faithfully reconstructed by |
And update the safety documentation
folkertdev
force-pushed
the
unseal-va-arg-safe
branch
from
36b9646 to
f986965
Compare
Why do we need to bound future designs which attempt to provide more assurance by what was justifiable at earlier implementation times by expedience? The initial approach did not think we should be, but then attempted to inflict a very strict bound. We are trying to effectively weaken the restrictions to something reasonable, that provides a balance of utility and soundness, without simply sending everyone into the abyss to get flayed alive by wandering compilers wielding a thousand edge cases. A lint generally signals something that we do not like. Often, something that we would design away if we could. Here, the API is relatively green, even if it is adjacent to some quite scorched fields. We could do even better hypothetically but that would require a lot more work. |
|
Hmm, trying a custom implementation with e.g. a So this is going to be more work, especially given the matrix of targets and calling conventions. |
|
Honestly I kind of think we shouldn't open this can of worms for the MVP of stable c-variadics.There is always |
|
closing in favor of #146521 |
rustbot
removed
the
S-waiting-on-review
…=workingjubilee document `core::ffi::VaArgSafe` tracking issue: rust-lang#44930 A modification of rust-lang#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`
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`
tracking issue: #44930
We concluded that this trait should be implementable by end users. Many ABIs do support passing additional types (128-bit integers, SIMD vectors, composite types).
r? @workingjubilee