about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJacob Pratt <jacob@jhpratt.dev>2025-05-25 04:00:57 +0200
committerGitHub <noreply@github.com>2025-05-25 04:00:57 +0200
commitc27b7c22148e1666633d6a19826c37fcece07569 (patch)
treead91cc843609eb47f88ab937a3620dc025572a93
parent3338ff7dcf1b038887532648e49240f6f5a03d07 (diff)
parent89a8abc4bea115c3508ee5ac4eb9ac3a55920316 (diff)
downloadrust-c27b7c22148e1666633d6a19826c37fcece07569.tar.gz
rust-c27b7c22148e1666633d6a19826c37fcece07569.zip
Rollup merge of #141361 - folkertdev:varargs-cfg, r=workingjubilee
use `cfg_select!` to select the right `VaListImpl` definition

tracking issue: https://github.com/rust-lang/rust/issues/44930

Just a bit of cleanup really.

We could use `PhantomInvariantLifetime<'f>` (https://github.com/rust-lang/rust/issues/135806) to make it more precise what that `PhantomData<&'f mut &'f c_void>` marker is doing. I'm not sure how ready that feature is though, `@jhpratt` are these types good to use internally?

---

Some research into the lifetimes of `VaList` and `VaListImpl`:

It's easy to see why the lifetime of these types should not be extended, a `VaList` or `VaListImpl` escaping its function is a bad idea. I don't currently see why coercing the lifetime to a shorter lifetime is problematic though, but probably I just don't understand variance well enough to see it. The history does not provide much explanation:

- https://github.com/immunant/rust/commit/08140878fefaa4b16939b904bf825b7107069b42 original implementation
- https://github.com/immunant/rust/commit/b9ea653aee231114acbe6d4b3c7b1d692772d060 adds `VaListImpl<'f>`, but it is only covariant in `'f`
- https://github.com/rust-lang/rust/pull/62639 makes `VaListImpl<'f>` invariant over `'f` (because `VaList<'a, 'f>` is already invariant over `'f`, but I think that is just an implementation detail?)

Beyond that I don't see how the lifetime situation can be simplified significantly, e.g. this function really needs `'copy` to be unconstrained.

```rust
/// Copies the `va_list` at the current location.
pub unsafe fn with_copy<F, R>(&self, f: F) -> R
where
    F: for<'copy> FnOnce(VaList<'copy, 'f>) -> R,
{
    let mut ap = self.clone();
    let ret = f(ap.as_va_list());
    // SAFETY: the caller must uphold the safety contract for `va_end`.
    unsafe {
        va_end(&mut ap);
    }
    ret
}
```

`@rustbot` label +F-c_variadic
r? `@workingjubilee`
-rw-r--r--library/core/src/ffi/va_list.rs311
1 files changed, 136 insertions, 175 deletions
diff --git a/library/core/src/ffi/va_list.rs b/library/core/src/ffi/va_list.rs
index f12bd289f27..8f7c090bc1b 100644
--- a/library/core/src/ffi/va_list.rs
+++ b/library/core/src/ffi/va_list.rs
@@ -5,148 +5,120 @@
 use crate::ffi::c_void;
 #[allow(unused_imports)]
 use crate::fmt;
-use crate::marker::PhantomData;
+use crate::marker::{PhantomData, PhantomInvariantLifetime};
 use crate::ops::{Deref, DerefMut};
 
-/// Basic implementation of a `va_list`.
 // The name is WIP, using `VaListImpl` for now.
-#[cfg(any(
+//
+// Most targets explicitly specify the layout of `va_list`, this layout is matched here.
+crate::cfg_select! {
     all(
-        not(target_arch = "aarch64"),
-        not(target_arch = "powerpc"),
-        not(target_arch = "s390x"),
-        not(target_arch = "xtensa"),
-        not(target_arch = "x86_64")
-    ),
-    all(target_arch = "aarch64", target_vendor = "apple"),
-    target_family = "wasm",
-    target_os = "uefi",
-    windows,
-))]
-#[repr(transparent)]
-#[lang = "va_list"]
-pub struct VaListImpl<'f> {
-    ptr: *mut c_void,
-
-    // Invariant over `'f`, so each `VaListImpl<'f>` object is tied to
-    // the region of the function it's defined in
-    _marker: PhantomData<&'f mut &'f c_void>,
-}
-
-#[cfg(any(
-    all(
-        not(target_arch = "aarch64"),
-        not(target_arch = "powerpc"),
-        not(target_arch = "s390x"),
-        not(target_arch = "xtensa"),
-        not(target_arch = "x86_64")
-    ),
-    all(target_arch = "aarch64", target_vendor = "apple"),
-    target_family = "wasm",
-    target_os = "uefi",
-    windows,
-))]
-impl<'f> fmt::Debug for VaListImpl<'f> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "va_list* {:p}", self.ptr)
+        target_arch = "aarch64",
+        not(target_vendor = "apple"),
+        not(target_os = "uefi"),
+        not(windows),
+    ) => {
+        /// AArch64 ABI implementation of a `va_list`. See the
+        /// [AArch64 Procedure Call Standard] for more details.
+        ///
+        /// [AArch64 Procedure Call Standard]:
+        /// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
+        #[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
+        #[derive(Debug)]
+        #[lang = "va_list"]
+        pub struct VaListImpl<'f> {
+            stack: *mut c_void,
+            gr_top: *mut c_void,
+            vr_top: *mut c_void,
+            gr_offs: i32,
+            vr_offs: i32,
+            _marker: PhantomInvariantLifetime<'f>,
+        }
+    }
+    all(target_arch = "powerpc", not(target_os = "uefi"), not(windows)) => {
+        /// PowerPC ABI implementation of a `va_list`.
+        #[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
+        #[derive(Debug)]
+        #[lang = "va_list"]
+        pub struct VaListImpl<'f> {
+            gpr: u8,
+            fpr: u8,
+            reserved: u16,
+            overflow_arg_area: *mut c_void,
+            reg_save_area: *mut c_void,
+            _marker: PhantomInvariantLifetime<'f>,
+        }
+    }
+    target_arch = "s390x" => {
+        /// s390x ABI implementation of a `va_list`.
+        #[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
+        #[derive(Debug)]
+        #[lang = "va_list"]
+        pub struct VaListImpl<'f> {
+            gpr: i64,
+            fpr: i64,
+            overflow_arg_area: *mut c_void,
+            reg_save_area: *mut c_void,
+            _marker: PhantomInvariantLifetime<'f>,
+        }
+    }
+    all(target_arch = "x86_64", not(target_os = "uefi"), not(windows)) => {
+        /// x86_64 ABI implementation of a `va_list`.
+        #[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
+        #[derive(Debug)]
+        #[lang = "va_list"]
+        pub struct VaListImpl<'f> {
+            gp_offset: i32,
+            fp_offset: i32,
+            overflow_arg_area: *mut c_void,
+            reg_save_area: *mut c_void,
+            _marker: PhantomInvariantLifetime<'f>,
+        }
+    }
+    target_arch = "xtensa" => {
+        /// Xtensa ABI implementation of a `va_list`.
+        #[repr(C)]
+        #[derive(Debug)]
+        #[lang = "va_list"]
+        pub struct VaListImpl<'f> {
+            stk: *mut i32,
+            reg: *mut i32,
+            ndx: i32,
+            _marker: PhantomInvariantLifetime<'f>,
+        }
     }
-}
-
-/// AArch64 ABI implementation of a `va_list`. See the
-/// [AArch64 Procedure Call Standard] for more details.
-///
-/// [AArch64 Procedure Call Standard]:
-/// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
-#[cfg(all(
-    target_arch = "aarch64",
-    not(target_vendor = "apple"),
-    not(target_os = "uefi"),
-    not(windows),
-))]
-#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
-#[derive(Debug)]
-#[lang = "va_list"]
-pub struct VaListImpl<'f> {
-    stack: *mut c_void,
-    gr_top: *mut c_void,
-    vr_top: *mut c_void,
-    gr_offs: i32,
-    vr_offs: i32,
-    _marker: PhantomData<&'f mut &'f c_void>,
-}
-
-/// PowerPC ABI implementation of a `va_list`.
-#[cfg(all(target_arch = "powerpc", not(target_os = "uefi"), not(windows)))]
-#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
-#[derive(Debug)]
-#[lang = "va_list"]
-pub struct VaListImpl<'f> {
-    gpr: u8,
-    fpr: u8,
-    reserved: u16,
-    overflow_arg_area: *mut c_void,
-    reg_save_area: *mut c_void,
-    _marker: PhantomData<&'f mut &'f c_void>,
-}
-
-/// s390x ABI implementation of a `va_list`.
-#[cfg(target_arch = "s390x")]
-#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
-#[derive(Debug)]
-#[lang = "va_list"]
-pub struct VaListImpl<'f> {
-    gpr: i64,
-    fpr: i64,
-    overflow_arg_area: *mut c_void,
-    reg_save_area: *mut c_void,
-    _marker: PhantomData<&'f mut &'f c_void>,
-}
 
-/// x86_64 ABI implementation of a `va_list`.
-#[cfg(all(target_arch = "x86_64", not(target_os = "uefi"), not(windows)))]
-#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
-#[derive(Debug)]
-#[lang = "va_list"]
-pub struct VaListImpl<'f> {
-    gp_offset: i32,
-    fp_offset: i32,
-    overflow_arg_area: *mut c_void,
-    reg_save_area: *mut c_void,
-    _marker: PhantomData<&'f mut &'f c_void>,
-}
+    // The fallback implementation, used for:
+    //
+    // - apple aarch64 (see https://github.com/rust-lang/rust/pull/56599)
+    // - windows
+    // - uefi
+    // - any other target for which we don't specify the `VaListImpl` above
+    //
+    // In this implementation the `va_list` type is just an alias for an opaque pointer.
+    // That pointer is probably just the next variadic argument on the caller's stack.
+    _ => {
+        /// Basic implementation of a `va_list`.
+        #[repr(transparent)]
+        #[lang = "va_list"]
+        pub struct VaListImpl<'f> {
+            ptr: *mut c_void,
+
+            // Invariant over `'f`, so each `VaListImpl<'f>` object is tied to
+            // the region of the function it's defined in
+            _marker: PhantomInvariantLifetime<'f>,
+        }
 
-/// Xtensa ABI implementation of a `va_list`.
-#[cfg(target_arch = "xtensa")]
-#[repr(C)]
-#[derive(Debug)]
-#[lang = "va_list"]
-pub struct VaListImpl<'f> {
-    stk: *mut i32,
-    reg: *mut i32,
-    ndx: i32,
-    _marker: PhantomData<&'f mut &'f c_void>,
+        impl<'f> fmt::Debug for VaListImpl<'f> {
+            fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+                write!(f, "va_list* {:p}", self.ptr)
+            }
+        }
+    }
 }
 
-/// A wrapper for a `va_list`
-#[repr(transparent)]
-#[derive(Debug)]
-pub struct VaList<'a, 'f: 'a> {
-    #[cfg(any(
-        all(
-            not(target_arch = "aarch64"),
-            not(target_arch = "powerpc"),
-            not(target_arch = "s390x"),
-            not(target_arch = "x86_64")
-        ),
-        target_arch = "xtensa",
-        all(target_arch = "aarch64", target_vendor = "apple"),
-        target_family = "wasm",
-        target_os = "uefi",
-        windows,
-    ))]
-    inner: VaListImpl<'f>,
-
-    #[cfg(all(
+crate::cfg_select! {
+    all(
         any(
             target_arch = "aarch64",
             target_arch = "powerpc",
@@ -158,52 +130,41 @@ pub struct VaList<'a, 'f: 'a> {
         not(target_family = "wasm"),
         not(target_os = "uefi"),
         not(windows),
-    ))]
-    inner: &'a mut VaListImpl<'f>,
+    ) => {
+        /// A wrapper for a `va_list`
+        #[repr(transparent)]
+        #[derive(Debug)]
+        pub struct VaList<'a, 'f: 'a> {
+            inner: &'a mut VaListImpl<'f>,
+            _marker: PhantomData<&'a mut VaListImpl<'f>>,
+        }
 
-    _marker: PhantomData<&'a mut VaListImpl<'f>>,
-}
 
-#[cfg(any(
-    all(
-        not(target_arch = "aarch64"),
-        not(target_arch = "powerpc"),
-        not(target_arch = "s390x"),
-        not(target_arch = "x86_64")
-    ),
-    target_arch = "xtensa",
-    all(target_arch = "aarch64", target_vendor = "apple"),
-    target_family = "wasm",
-    target_os = "uefi",
-    windows,
-))]
-impl<'f> VaListImpl<'f> {
-    /// Converts a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
-    #[inline]
-    pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
-        VaList { inner: VaListImpl { ..*self }, _marker: PhantomData }
+        impl<'f> VaListImpl<'f> {
+            /// Converts a [`VaListImpl`] into a [`VaList`] that is binary-compatible with C's `va_list`.
+            #[inline]
+            pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
+                VaList { inner: self, _marker: PhantomData }
+            }
+        }
     }
-}
 
-#[cfg(all(
-    any(
-        target_arch = "aarch64",
-        target_arch = "powerpc",
-        target_arch = "s390x",
-        target_arch = "xtensa",
-        target_arch = "x86_64"
-    ),
-    not(target_arch = "xtensa"),
-    any(not(target_arch = "aarch64"), not(target_vendor = "apple")),
-    not(target_family = "wasm"),
-    not(target_os = "uefi"),
-    not(windows),
-))]
-impl<'f> VaListImpl<'f> {
-    /// Converts a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
-    #[inline]
-    pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
-        VaList { inner: self, _marker: PhantomData }
+    _ => {
+        /// A wrapper for a `va_list`
+        #[repr(transparent)]
+        #[derive(Debug)]
+        pub struct VaList<'a, 'f: 'a> {
+            inner: VaListImpl<'f>,
+            _marker: PhantomData<&'a mut VaListImpl<'f>>,
+        }
+
+        impl<'f> VaListImpl<'f> {
+            /// Converts a [`VaListImpl`] into a [`VaList`] that is binary-compatible with C's `va_list`.
+            #[inline]
+            pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
+                VaList { inner: VaListImpl { ..*self }, _marker: PhantomData }
+            }
+        }
     }
 }