From 9a052c3442a50d00cb4898da0087d7d79cec23d4 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 9 Mar 2026 09:13:46 +0100 Subject: [PATCH] replace shadowed return token by `unsafe`-to-create token We use a unit struct `__InitOk` in the closure generated by the initializer macros as the return value. We shadow it by creating a struct with the same name again inside of the closure, preventing early returns of `Ok` in the initializer (before all fields have been initialized). In the face of Type Alias Impl Trait (TAIT) and the next trait solver, this solution no longer works [1]. The shadowed struct can be named through type inference. In addition, there is an RFC proposing to add the feature of path inference to Rust, which would similarly allow [2] Thus remove the shadowed token and replace it with an `unsafe` to create token. The reason we initially used the shadowing solution was because an alternative solution used a builder pattern. Gary writes [3]: In the early builder-pattern based InitOk, having a single InitOk type for token is unsound because one can launder an InitOk token used for one place to another initializer. I used a branded lifetime solution, and then you figured out that using a shadowed type would work better because nobody could construct it at all. The laundering issue does not apply to the approach we ended up with today. With this change, the example by Tim Chirananthavat in [1] no longer compiles and results in this error: error: cannot construct `pin_init::__internal::InitOk` with struct literal syntax due to private fields --> src/main.rs:26:17 | 26 | InferredType {} | ^^^^^^^^^^^^ | = note: private field `0` that was not provided help: you might have meant to use the `new` associated function | 26 - InferredType {} 26 + InferredType::new() | Applying the suggestion of using the `::new()` function, results in another expected error: error[E0133]: call to unsafe function `pin_init::__internal::InitOk::new` is unsafe and requires unsafe block --> src/main.rs:26:17 | 26 | InferredType::new() | ^^^^^^^^^^^^^^^^^^^ call to unsafe function | = note: consult the function's documentation for information on how to avoid undefined behavior Reported-by: Tim Chirananthavat Link: https://github.com/rust-lang/rust/issues/153535 [1] Link: https://github.com/rust-lang/rfcs/pull/3444#issuecomment-4016145373 [2] Link: https://github.com/rust-lang/rust/issues/153535#issuecomment-4017620804 [3] Signed-off-by: Benno Lossin --- internal/src/init.rs | 22 ++---- src/__internal.rs | 28 ++++++- tests/ui/compile-fail/init/early_return.rs | 14 ++++ .../ui/compile-fail/init/early_return.stderr | 36 +++++++++ tests/ui/expand/no_field_access.expanded.rs | 73 +++++++++---------- tests/ui/expand/simple-init.expanded.rs | 11 +-- 6 files changed, 118 insertions(+), 66 deletions(-) create mode 100644 tests/ui/compile-fail/init/early_return.rs create mode 100644 tests/ui/compile-fail/init/early_return.stderr diff --git a/internal/src/init.rs b/internal/src/init.rs index 1a1e3cb4..3cde113d 100644 --- a/internal/src/init.rs +++ b/internal/src/init.rs @@ -156,11 +156,6 @@ pub(crate) fn expand( ); let field_check = make_field_check(&fields, init_kind, &path); Ok(quote! {{ - // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return - // type and shadow it later when we insert the arbitrary user code. That way there will be - // no possibility of returning without `unsafe`. - struct __InitOk; - // Get the data about fields from the supplied type. // SAFETY: TODO let #data = unsafe { @@ -170,18 +165,15 @@ pub(crate) fn expand( #path::#get_data() }; // Ensure that `#data` really is of type `#data` and help with type inference: - let init = ::pin_init::__internal::#data_trait::make_closure::<_, __InitOk, #error>( + let init = ::pin_init::__internal::#data_trait::make_closure::<_, #error>( #data, move |slot| { - { - // Shadow the structure so it cannot be used to return early. - struct __InitOk; - #zeroable_check - #this - #init_fields - #field_check - } - Ok(__InitOk) + #zeroable_check + #this + #init_fields + #field_check + // SAFETY: we are the `init!` macro that is allowed to call this. + Ok(unsafe { ::pin_init::__internal::InitOk::new() }) } ); let init = move |slot| -> ::core::result::Result<(), #error> { diff --git a/src/__internal.rs b/src/__internal.rs index 90f18e9a..90adbdc1 100644 --- a/src/__internal.rs +++ b/src/__internal.rs @@ -46,6 +46,24 @@ where } } +/// Token type to signify successful initialization. +/// +/// Can only be constructed via the unsafe [`Self::new`] function. The initializer macros use this +/// token type to prevent returning `Ok` from an initializer without initializing all fields. +pub struct InitOk(()); + +impl InitOk { + /// Creates a new token. + /// + /// # Safety + /// + /// This function may only be called from the `init!` macro in `../internal/src/init.rs`. + #[inline(always)] + pub unsafe fn new() -> Self { + Self(()) + } +} + /// This trait is only implemented via the `#[pin_data]` proc-macro. It is used to facilitate /// the pin projections within the initializers. /// @@ -68,9 +86,10 @@ pub unsafe trait PinData: Copy { type Datee: ?Sized + HasPinData; /// Type inference helper function. - fn make_closure(self, f: F) -> F + #[inline(always)] + fn make_closure(self, f: F) -> F where - F: FnOnce(*mut Self::Datee) -> Result, + F: FnOnce(*mut Self::Datee) -> Result, { f } @@ -98,9 +117,10 @@ pub unsafe trait InitData: Copy { type Datee: ?Sized + HasInitData; /// Type inference helper function. - fn make_closure(self, f: F) -> F + #[inline(always)] + fn make_closure(self, f: F) -> F where - F: FnOnce(*mut Self::Datee) -> Result, + F: FnOnce(*mut Self::Datee) -> Result, { f } diff --git a/tests/ui/compile-fail/init/early_return.rs b/tests/ui/compile-fail/init/early_return.rs new file mode 100644 index 00000000..5f982b7c --- /dev/null +++ b/tests/ui/compile-fail/init/early_return.rs @@ -0,0 +1,14 @@ +use pin_init::*; + +struct Foo { + a: usize, +} + +fn main() { + let _ = init!(Foo { + _: { + return Ok(()); + }, + a: 42, + }); +} diff --git a/tests/ui/compile-fail/init/early_return.stderr b/tests/ui/compile-fail/init/early_return.stderr new file mode 100644 index 00000000..82d56a08 --- /dev/null +++ b/tests/ui/compile-fail/init/early_return.stderr @@ -0,0 +1,36 @@ +error[E0308]: mismatched types + --> tests/ui/compile-fail/init/early_return.rs:10:23 + | +10 | return Ok(()); + | -- ^^ expected `InitOk`, found `()` + | | + | arguments to this enum variant are incorrect + | +help: the type constructed contains `()` due to the type of the argument passed + --> tests/ui/compile-fail/init/early_return.rs:10:20 + | +10 | return Ok(()); + | ^^^--^ + | | + | this argument influences the type of `Ok` +note: tuple variant defined here + --> $RUST/core/src/result.rs + | + | Ok(#[stable(feature = "rust1", since = "1.0.0")] T), + | ^^ + +warning: unreachable statement + --> tests/ui/compile-fail/init/early_return.rs:8:13 + | + 8 | let _ = init!(Foo { + | _____________^ + 9 | | _: { +10 | | return Ok(()); + | | ------------- any code following this expression is unreachable +11 | | }, +12 | | a: 42, +13 | | }); + | |______^ unreachable statement + | + = note: `#[warn(unreachable_code)]` (part of `#[warn(unused)]`) on by default + = note: this warning originates in the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/expand/no_field_access.expanded.rs b/tests/ui/expand/no_field_access.expanded.rs index 2b935432..e4df59eb 100644 --- a/tests/ui/expand/no_field_access.expanded.rs +++ b/tests/ui/expand/no_field_access.expanded.rs @@ -7,57 +7,52 @@ struct Foo { } fn main() { let _ = { - struct __InitOk; let __data = unsafe { use ::pin_init::__internal::HasInitData; Foo::__init_data() }; let init = ::pin_init::__internal::InitData::make_closure::< _, - __InitOk, ::core::convert::Infallible, >( __data, move |slot| { { - struct __InitOk; - { - let c = -42; - unsafe { ::core::ptr::write(&raw mut (*slot).c, c) }; - } - let __c_guard = unsafe { - ::pin_init::__internal::DropGuard::new(&raw mut (*slot).c) - }; - { - let b = *c; - unsafe { ::core::ptr::write(&raw mut (*slot).b, b) }; - } - let __b_guard = unsafe { - ::pin_init::__internal::DropGuard::new(&raw mut (*slot).b) - }; - { - let a = 0; - unsafe { ::core::ptr::write(&raw mut (*slot).a, a) }; - } - let __a_guard = unsafe { - ::pin_init::__internal::DropGuard::new(&raw mut (*slot).a) - }; - ::core::mem::forget(__c_guard); - ::core::mem::forget(__b_guard); - ::core::mem::forget(__a_guard); - #[allow(unreachable_code, clippy::diverging_sub_expression)] - let _ = || unsafe { - ::core::ptr::write( - slot, - Foo { - c: ::core::panicking::panic("explicit panic"), - b: ::core::panicking::panic("explicit panic"), - a: ::core::panicking::panic("explicit panic"), - }, - ) - }; + let c = -42; + unsafe { ::core::ptr::write(&raw mut (*slot).c, c) }; } - Ok(__InitOk) + let __c_guard = unsafe { + ::pin_init::__internal::DropGuard::new(&raw mut (*slot).c) + }; + { + let b = *c; + unsafe { ::core::ptr::write(&raw mut (*slot).b, b) }; + } + let __b_guard = unsafe { + ::pin_init::__internal::DropGuard::new(&raw mut (*slot).b) + }; + { + let a = 0; + unsafe { ::core::ptr::write(&raw mut (*slot).a, a) }; + } + let __a_guard = unsafe { + ::pin_init::__internal::DropGuard::new(&raw mut (*slot).a) + }; + ::core::mem::forget(__c_guard); + ::core::mem::forget(__b_guard); + ::core::mem::forget(__a_guard); + #[allow(unreachable_code, clippy::diverging_sub_expression)] + let _ = || unsafe { + ::core::ptr::write( + slot, + Foo { + c: ::core::panicking::panic("explicit panic"), + b: ::core::panicking::panic("explicit panic"), + a: ::core::panicking::panic("explicit panic"), + }, + ) + }; + Ok(unsafe { ::pin_init::__internal::InitOk::new() }) }, ); let init = move | diff --git a/tests/ui/expand/simple-init.expanded.rs b/tests/ui/expand/simple-init.expanded.rs index 822d84c8..d2fd17b0 100644 --- a/tests/ui/expand/simple-init.expanded.rs +++ b/tests/ui/expand/simple-init.expanded.rs @@ -2,24 +2,19 @@ use pin_init::*; struct Foo {} fn main() { let _ = { - struct __InitOk; let __data = unsafe { use ::pin_init::__internal::HasInitData; Foo::__init_data() }; let init = ::pin_init::__internal::InitData::make_closure::< _, - __InitOk, ::core::convert::Infallible, >( __data, move |slot| { - { - struct __InitOk; - #[allow(unreachable_code, clippy::diverging_sub_expression)] - let _ = || unsafe { ::core::ptr::write(slot, Foo {}) }; - } - Ok(__InitOk) + #[allow(unreachable_code, clippy::diverging_sub_expression)] + let _ = || unsafe { ::core::ptr::write(slot, Foo {}) }; + Ok(unsafe { ::pin_init::__internal::InitOk::new() }) }, ); let init = move |