Skip to content

Commit 0d5cd7f

Browse files
committed
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 <theemathas@gmail.com> Link: rust-lang/rust#153535 [1] Link: rust-lang/rfcs#3444 (comment) [2] Link: rust-lang/rust#153535 (comment) [3] Signed-off-by: Benno Lossin <lossin@kernel.org>
1 parent b4c9632 commit 0d5cd7f

5 files changed

Lines changed: 84 additions & 27 deletions

File tree

internal/src/init.rs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,6 @@ pub(crate) fn expand(
148148
let init_fields = init_fields(&fields, pinned, &data, &slot);
149149
let field_check = make_field_check(&fields, init_kind, &path);
150150
Ok(quote! {{
151-
// We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
152-
// type and shadow it later when we insert the arbitrary user code. That way there will be
153-
// no possibility of returning without `unsafe`.
154-
struct __InitOk;
155-
156151
// Get the data about fields from the supplied type.
157152
// SAFETY: TODO
158153
let #data = unsafe {
@@ -162,18 +157,15 @@ pub(crate) fn expand(
162157
#path::#get_data()
163158
};
164159
// Ensure that `#data` really is of type `#data` and help with type inference:
165-
let init = ::pin_init::__internal::#data_trait::make_closure::<_, __InitOk, #error>(
160+
let init = ::pin_init::__internal::#data_trait::make_closure::<_, #error>(
166161
#data,
167162
move |slot| {
168-
{
169-
// Shadow the structure so it cannot be used to return early.
170-
struct __InitOk;
171-
#zeroable_check
172-
#this
173-
#init_fields
174-
#field_check
175-
}
176-
Ok(__InitOk)
163+
#zeroable_check
164+
#this
165+
#init_fields
166+
#field_check
167+
// SAFETY: we are the `init!` macro that is allowed to call this.
168+
Ok(unsafe { ::pin_init::__internal::InitOk::new() })
177169
}
178170
);
179171
let init = move |slot| -> ::core::result::Result<(), #error> {

src/__internal.rs

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,24 @@ where
4646
}
4747
}
4848

49+
/// Token type to signify successful initialization.
50+
///
51+
/// Can only be constructed via the unsafe [`Self::new`] function. The initializer macros use this
52+
/// token type to prevent returning `Ok` from an initializer without initializing all fields.
53+
pub struct InitOk(());
54+
55+
impl InitOk {
56+
/// Creates a new token.
57+
///
58+
/// # Safety
59+
///
60+
/// This function may only be called from the `init!` macro in `../internal/src/init.rs`.
61+
#[inline(always)]
62+
pub unsafe fn new() -> Self {
63+
Self(())
64+
}
65+
}
66+
4967
/// This trait is only implemented via the `#[pin_data]` proc-macro. It is used to facilitate
5068
/// the pin projections within the initializers.
5169
///
@@ -68,9 +86,10 @@ pub unsafe trait PinData: Copy {
6886
type Datee: ?Sized + HasPinData;
6987

7088
/// Type inference helper function.
71-
fn make_closure<F, O, E>(self, f: F) -> F
89+
#[inline(always)]
90+
fn make_closure<F, E>(self, f: F) -> F
7291
where
73-
F: FnOnce(*mut Self::Datee) -> Result<O, E>,
92+
F: FnOnce(*mut Self::Datee) -> Result<InitOk, E>,
7493
{
7594
f
7695
}
@@ -98,9 +117,10 @@ pub unsafe trait InitData: Copy {
98117
type Datee: ?Sized + HasInitData;
99118

100119
/// Type inference helper function.
101-
fn make_closure<F, O, E>(self, f: F) -> F
120+
#[inline(always)]
121+
fn make_closure<F, E>(self, f: F) -> F
102122
where
103-
F: FnOnce(*mut Self::Datee) -> Result<O, E>,
123+
F: FnOnce(*mut Self::Datee) -> Result<InitOk, E>,
104124
{
105125
f
106126
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
use pin_init::*;
2+
3+
struct Foo {
4+
a: usize,
5+
}
6+
7+
fn main() {
8+
let _ = init!(Foo {
9+
_: {
10+
return Ok(());
11+
},
12+
a: 42,
13+
});
14+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
error[E0308]: mismatched types
2+
--> tests/ui/compile-fail/init/early_return.rs:10:23
3+
|
4+
10 | return Ok(());
5+
| -- ^^ expected `InitOk`, found `()`
6+
| |
7+
| arguments to this enum variant are incorrect
8+
|
9+
help: the type constructed contains `()` due to the type of the argument passed
10+
--> tests/ui/compile-fail/init/early_return.rs:10:20
11+
|
12+
10 | return Ok(());
13+
| ^^^--^
14+
| |
15+
| this argument influences the type of `Ok`
16+
note: tuple variant defined here
17+
--> $RUST/core/src/result.rs
18+
|
19+
| Ok(#[stable(feature = "rust1", since = "1.0.0")] T),
20+
| ^^
21+
22+
warning: unreachable statement
23+
--> tests/ui/compile-fail/init/early_return.rs:8:13
24+
|
25+
8 | let _ = init!(Foo {
26+
| _____________^
27+
9 | | _: {
28+
10 | | return Ok(());
29+
| | ------------- any code following this expression is unreachable
30+
11 | | },
31+
12 | | a: 42,
32+
13 | | });
33+
| |______^ unreachable statement
34+
|
35+
= note: `#[warn(unreachable_code)]` (part of `#[warn(unused)]`) on by default
36+
= note: this warning originates in the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info)

tests/ui/expand/simple-init.expanded.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,19 @@ use pin_init::*;
22
struct Foo {}
33
fn main() {
44
let _ = {
5-
struct __InitOk;
65
let __data = unsafe {
76
use ::pin_init::__internal::HasInitData;
87
Foo::__init_data()
98
};
109
let init = ::pin_init::__internal::InitData::make_closure::<
1110
_,
12-
__InitOk,
1311
::core::convert::Infallible,
1412
>(
1513
__data,
1614
move |slot| {
17-
{
18-
struct __InitOk;
19-
#[allow(unreachable_code, clippy::diverging_sub_expression)]
20-
let _ = || unsafe { ::core::ptr::write(slot, Foo {}) };
21-
}
22-
Ok(__InitOk)
15+
#[allow(unreachable_code, clippy::diverging_sub_expression)]
16+
let _ = || unsafe { ::core::ptr::write(slot, Foo {}) };
17+
Ok(unsafe { ::pin_init::__internal::InitOk::new() })
2318
},
2419
);
2520
let init = move |

0 commit comments

Comments
 (0)