Skip to content

replace shadowed return token by unsafe-to-create token#117

Open
BennoLossin wants to merge 1 commit intomainfrom
dev/unsound-fix-TAIT-next-solver
Open

replace shadowed return token by unsafe-to-create token#117
BennoLossin wants to merge 1 commit intomainfrom
dev/unsound-fix-TAIT-next-solver

Conversation

@BennoLossin
Copy link
Member

@BennoLossin BennoLossin commented Mar 9, 2026

@theemathas let me know if you're okay with me listing you as the Reported-by (or if I should use a different email).

I wanted to include your example from the issue to ensure that we don't forget about this in the future, but I will have to add a way to run ui tests with compiler flags, as trybuild doesn't natively support that.


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]

@theemathas
Copy link

This email is fine 👍

@BennoLossin
Copy link
Member Author

BennoLossin commented Mar 9, 2026

Perfect. Thanks a lot for catching this issue!

@BennoLossin BennoLossin force-pushed the dev/unsound-fix-TAIT-next-solver branch from 40bc552 to e981f14 Compare March 9, 2026 12:23
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>
@BennoLossin BennoLossin force-pushed the dev/unsound-fix-TAIT-next-solver branch from e981f14 to 9a052c3 Compare March 9, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants