Consider folkertdev's `c_variadic` proposal

[traviscross]

Over in #44930, @folkertdev proposed:

We need c_variadics on stable for zlib-rs, so I'm going to try and move this forward. A draft for a modified API is at

https://gist.github.com/folkertdev/47c79e2f5b03f1138db9d53be5e51ed1

Some highlights

• VaListImpl is private!
• VaList just has a single lifetime parameter now, which makes a lot more sense
• The VaList::with_copy function and a new va_copy! macro can be used to duplicate a VaList

This all relies on a different desugaring of ..., roughly

fn foo(placeholder: i32, mut args: ...) {
    // stuff
}

// --->

fn foo(placeholder: i32) {
    let tag = core::mem::MaybeUninit::<VaListImpl>::uninit();
    unsafe { core::instrinsics::va_start(tag.as_mut_ptr()) }
    let mut args = VaList(tag.assume_init_mut())

    // stuff
}

It seems possible to make that work internally.

---

I think this new API is nicer, because it leaks fewer implementation details. But before we start implementation work, does anyone see issues with this direction?

pinging specifically the c2rust folks @thedataking @ahomescu, you've probably worked with the existing c_variadics most.

Thoughts?

Tracking:

• Tracking issue for RFC 2137: Support defining C-compatible variadic functions in Rust (c_variadic) #44930

cc @rust-lang/lang

[folkertdev]

To explore these ideas, I've implemented my proposal in ea75827.

This is prototype quality at the moment, but I think the examples/tests look much better now that the only user-visible type is VaList. It all seems to work (well, after I implemented #141538 to stop LLVM miscompiling va_arg) and generates the expected (lifetime) errors.

I think my current proposal is more T-libs than T-lang, but c_variadics in general can probably use your discussion: most work on them is from 5+ years ago.

For T-lang specifically:

• are you OK with further use of super let, similar to the pin! macro.
• this sort of touches on 'fn lifetimes, though my proposal does not need it, because the logic now gives the VaList the lifetime of a local binding on the stack
• not part of my proposal, but relevant: should arguments to variadic functions be constrained by VaArgSafe to fix Varargs are completely unchecked if passed as generics #61275
• is VaArgSafe too limited: currently bool, 8-bit and 16-bit integers, and f32 don't implement this trait, because in C these types will (silently) participate in implicit promotion. That does mean that users/tools that want to pass or receive these types have to perform this casting themselves. We already do have good errors for these cases, and and a lint that suggests the right cast.

For T-libs when the time comes

• The va_copy macro requires a doc(hidden) function that does most of the actual work.
• That hidden function does some ugly unsafe stuff so that we don't need to expose VaListTag (formerly VaListImpl, the "tag" mirrors the clang/llvm name). I believe that it's sound, but it does look weird, maybe there is some way to make that nicer that I'm missing

[workingjubilee]

The current proposal does not include i8, u8, etc. as VaArgSafe types on my advisory because of an important detail that may affect both this API and also, depending on what is decided, existing usage of ... by Rust code (namely: calling external functions with it).

• As folkertdev already mentioned, Varargs are completely unchecked if passed as generics #61275 shows that we do not check types are compatible with variable arguments when the type is generic. This allows an end-run around the check that enforces this effective bound, and later parts of the compiler miscompile the result.
• One of the obvious answers for how to solve this would be to move the check into the trait system and check for a trait. The obvious choice would be VaArgSafe, as that would mean we would have one trait, not two, but that militates against allowing it to do implicit "coercions" for smaller types like originally proposed.
• It is still possible to square this circle, allowing both "sides" to be described by a single trait and operate in the same way, if we allow implicit numeric promotion of arguments passed to .... I do not recommend adding numeric promotion to the language, even if it's hard to perceive in this case.
• The same code in C would simply not work for reasons that aren't obvious: even if you passed in a char on one side, you must ask for it as an int when using va_arg! Asking for char is UB because it was promoted to int! For floats, these conversions cannot be "simply" sign-extended and everything is a bit weirder, which is part of what provokes miscompilations.
• I think we should keep it simple and adhere rigidly to "what C allows, without C's conveniences"¹ because while it makes for annoying code in places, the simplicity allows for users to easily understand what happens. That would mean it would be possible to, in the future², allow user-guided implementation of the trait for their own types, especially including repr(C) structs that are ABI-stable enough for sensibly crossing FFI boundaries. These can be passed into ... and taken from va_arg in C, and in some libraries this ability is important.

While having VaList::arg work for structs is considered "out of scope" for the initial stabilization, there is also currently no other plan for how to make this work for VaList::arg in the future due to its trait bound. At least, not without just stripping the bound entirely. I believe adding even tiny amounts of added complexity to how C's variable arguments are handled by the compiler would threaten making the compiler's approach sufficiently illegible that it would be hard to understand when such an implementation would be valid in the future.

Footnotes

1. The "code brutalism" approach. ↩

2. The current va_arg implementation in the compiler isn't up-to-snuff to handle this yet, so this is a forward-looking comment only. ↩

[workingjubilee]

I found something more interesting while reviewing @folkertdev's PR #141538 which is that we currently decide the ABI we handle variable arguments for based on the target.

However, if we want our VaList type to have parity with the extended_varargs_abi_support feature, we need the ability to declare both extern "sysv64" fn(...) and extern "win64" fn(...) in the same program. This means we need those to expand to something that looks like VaList<'f, extern "sysv64"> and VaList<'f, extern "win64"> in the same program. If T-lang wishes us to support this... and I can't see why you wouldn't, as passing and receiving having parity seems valuable to me... then @folkertdev's proposal will have to be elaborated significantly.

So we need at least one more decision here from T-lang, though more of a confirmation:

• Do we need to support receiving C variable arguments, thus a ...-to-VaList desugaring, for every extern "abi" for which a target supports declaring

unsafe extern "abi" {
    fn uses_varargs(...);
}

Note that doing such will bring us afoul of this:

• VaList<'_> does not carry its ABI in its type #141618

[beetrees]

Note that modern C can pass f32/u8/i8/u16/i16 without numeric promotion. For instance, float/short will both be promoted to double/int, whereas _Float32/_BitInt(16) will be passed without numeric promotion.

[workingjubilee]

@beetrees Well, yes, but short isn't the same type as _BitInt(16), and I'm not sure the _Float32 handling isn't a bug. The complexity of such cases seems more like evidence against doing things "for people".

[beetrees]

I don't think the _Float32 handling is a bug AFAICT: the C23 standard states in section 6.5.2.2 (this is from a recent draft as I don't have access to the final standard) "The integer promotions are performed on each trailing argument,
and trailing arguments that have type float are promoted to double . These are called the default
argument promotions. No other conversions are performed implicitly". As _Float32 is a different type in the C type system (the same way that e.g. long and long long are always separate types even if they are the same width), it doesn't get promoted. Specifically, the standard removed default promotions for _FloatN types in a January/February 2022 Virtual Meeting.

The complexity of such cases seems more like evidence against doing things "for people".

I agree that Rust shouldn't be automatically promoting arguments for people.

[workingjubilee]

Fascinating. May have to rethink how this should be approached a bit.