Skip to content

[pointer] Support generic TransmuteFrom framework #2408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Feb 28, 2025

This commit removes the TransparentWrapper trait and the
Ptr::transparent_wrapper_into_inner method. It replaces them with a
new family of transmutation traits which encode more generic
transmutation (from any T to any U) and a set of Ptr methods which
use those traits to bound transmutation operations.

In particular:

  • Dst: TransmuteFrom<Src> denotes that a by-value transmutation is
    sound
  • Dst: TryTransmuteFromPtr<Src> denotes that a transmutation is sound
    so long as it can be guaranteed that the source is bit-valid for the
    destination; this is used by e.g. Ptr::try_into_valid, which
    performs runtime validation of bit validity
  • Dst: TransmuteFromPtr<Src> is equivalent to TransmuteFrom<Src> + TryTransmuteFromPtr<Src>

Some type arguments are omitted in this summary. In particular, all
three traits also take validity invariant parameters for both the source
and destination types. Also, the [Try]TransmuteFromPtr traits take an
aliasing parameter.

In order to support these traits, we introduce a generalization of
Read known as MutationCompatible. T: MutationCompatible<U, A>
denotes that either T: Read<A> and U: Read<A> or T and U
have the same interior mutation semantics (formally, it is sound for
&T and &U to reference the same referent - safe code operating on
these references cannot cause undefined behavior). This is a refinement
of the "UnsafeCell agreement" concept that we have used before, but it
supports types which store but don't actually use UnsafeCells. For
example, given a hypothetical ReadOnly<T>, the following bound holds:

usize: MutationCompatible<ReadOnly, Exclusive>

This commit also takes a different approach from the one originally
envisioned in #1945. In particular, it turns out that we don't need a
full type-level mapping concept. Instead, we need a predicate over
transitions to determine which ones are valid (e.g., it is valid to go
from a Valid MaybeUninit<T> to an Uninit MaybeUninit<T>). By
contrast, the invariant mapping concept suggests that each source
validity has exactly one destination validity.

This commit makes progress on #1940 by supporting unsized
transmutations, but we don't yet support size shrinking or expanding
transmutations.

This commit obsoletes #1359, as that issue was predicated upon the
existence of TransparentWrapper, which this commit removes.

This commit closes #1226, which suggests supporting UnsafeCell
agreement.

Closes #1945
Closes #1359
Closes #2226
Closes #1226
Closes #1866
Makes progress on #1359

Co-authored-by: Jack Wrenn [email protected]


This PR is on branch ptr-validity.

@joshlf
Copy link
Member Author

joshlf commented Feb 28, 2025

I think we may have to handle UnsafeCell agreement after all. In particular, in order to implement TryFromBytes for atomics, we need to be able to go from e.g. Ptr<AtomicU8> to Ptr<UnsafeCell<u8>> so that we can call <UnsafeCell<u8> as TryFromBytes>::is_bit_valid. This will require care since this is unsound for shared aliasing. We know in practice that we'll always be using exclusive aliasing, but I'm not sure how to express that generically.

@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch from 4bd4fd9 to f1b2bec Compare March 2, 2025 23:21
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch from 03547a1 to 0c59508 Compare March 2, 2025 23:21
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch from f1b2bec to 653cdb9 Compare March 2, 2025 23:46
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch from 653cdb9 to 3e52166 Compare March 3, 2025 00:02
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch from 0c59508 to daf3a21 Compare March 3, 2025 00:02
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch 6 times, most recently from c640f3e to 6151c97 Compare March 4, 2025 00:20
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch from 6151c97 to bbf07a8 Compare March 4, 2025 17:41
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch 8 times, most recently from b37fcfa to 3a43dfd Compare March 4, 2025 23:28
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch 2 times, most recently from 0c63965 to 9c88adc Compare March 4, 2025 23:54
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch from daf3a21 to c559aad Compare March 5, 2025 20:04
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch 5 times, most recently from a247151 to 8695084 Compare March 6, 2025 19:57
@joshlf joshlf force-pushed the Ie66db9044be1dc310a6b7280a73652a357878376 branch from c559aad to 9a5a4dd Compare March 6, 2025 19:57
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch 3 times, most recently from 079852a to 48e7eb3 Compare March 6, 2025 20:38
@joshlf joshlf changed the title [pointer][WIP] Transmute [pointer] Support generic TransmuteFrom framework Mar 6, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 6, 2025

Codecov Report

Attention: Patch coverage is 95.35865% with 11 lines in your changes missing coverage. Please review.

Project coverage is 88.03%. Comparing base (52f65db) to head (64a08b3).

Files with missing lines Patch % Lines
src/util/macros.rs 52.94% 8 Missing ⚠️
src/util/macro_util.rs 80.00% 2 Missing ⚠️
src/impls.rs 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2408      +/-   ##
==========================================
+ Coverage   87.33%   88.03%   +0.70%     
==========================================
  Files          17       17              
  Lines        6451     6412      -39     
==========================================
+ Hits         5634     5645      +11     
+ Misses        817      767      -50     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch from 48e7eb3 to 91aea96 Compare March 6, 2025 20:51
Copy link
Collaborator

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Developed and reviewed in tandem!

Base automatically changed from Ie66db9044be1dc310a6b7280a73652a357878376 to main March 6, 2025 21:25
This commit removes the `TransparentWrapper` trait and the
`Ptr::transparent_wrapper_into_inner` method. It replaces them with a
new family of transmutation traits which encode more generic
transmutation (from any `T` to any `U`) and a set of `Ptr` methods which
use those traits to bound transmutation operations.

In particular:
- `Dst: TransmuteFrom<Src>` denotes that a by-value transmutation is
  sound
- `Dst: TryTransmuteFromPtr<Src>` denotes that a transmutation is sound
  so long as it can be guaranteed that the source is bit-valid for the
  destination; this is used by e.g. `Ptr::try_into_valid`, which
  performs runtime validation of bit validity
- `Dst: TransmuteFromPtr<Src>` is equivalent to `TransmuteFrom<Src> +
  TryTransmuteFromPtr<Src>`

Some type arguments are omitted in this summary. In particular, all
three traits also take validity invariant parameters for both the source
and destination types. Also, the `[Try]TransmuteFromPtr` traits take an
aliasing parameter.

In order to support these traits, we introduce a generalization of
`Read` known as `MutationCompatible`. `T: MutationCompatible<U, A>`
denotes that *either* `T: Read<A>` and `U: Read<A>` *or* `T` and `U`
have the same interior mutation semantics (formally, it is sound for
`&T` and `&U` to reference the same referent - safe code operating on
these references cannot cause undefined behavior). This is a refinement
of the "`UnsafeCell` agreement" concept that we have used before, but it
supports types which store but don't actually use `UnsafeCell`s. For
example, given a hypothetical `ReadOnly<T>`, the following bound holds:

  usize: MutationCompatible<ReadOnly<AtomicUsize>, Exclusive>

This commit also takes a different approach from the one originally
envisioned in #1945. In particular, it turns out that we don't need a
full type-level mapping concept. Instead, we need a *predicate* over
transitions to determine which ones are valid (e.g., it is valid to go
from a `Valid` `MaybeUninit<T>` to an `Uninit` `MaybeUninit<T>`). By
contrast, the invariant mapping concept suggests that each source
validity has *exactly one* destination validity.

This commit makes progress on #1940 by supporting unsized
transmutations, but we don't yet support size shrinking or expanding
transmutations.

This commit obsoletes #1359, as that issue was predicated upon the
existence of `TransparentWrapper`, which this commit removes.

This commit closes #1226, which suggests supporting `UnsafeCell`
agreement.

Closes #1945
Closes #1359
Closes #2226
Closes #1226
Closes #1866
Makes progress on #1359

Co-authored-by: Jack Wrenn <[email protected]>
gherrit-pr-id: Iad14813bc6d933312bc8d7a1ddcf1aafc7126938
@joshlf joshlf force-pushed the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch from 91aea96 to 64a08b3 Compare March 6, 2025 21:26
@joshlf joshlf enabled auto-merge March 6, 2025 21:26
@joshlf joshlf added this pull request to the merge queue Mar 6, 2025
Merged via the queue into main with commit ee12c01 Mar 6, 2025
87 checks passed
@joshlf joshlf deleted the Iad14813bc6d933312bc8d7a1ddcf1aafc7126938 branch March 6, 2025 22:00
@joshlf joshlf mentioned this pull request Mar 6, 2025
joshlf added a commit that referenced this pull request Mar 6, 2025
In #2408, we simplified the safety precondition of `unsafe_impl!`, but
did not remove safety proofs at call sites made redundant by that
simplification. This commit removes those now-obsolete proofs.

gherrit-pr-id: I70d5aa5ace6bd2e39e679eac7f00a66d4b843d57
joshlf added a commit that referenced this pull request Mar 7, 2025
In #2408, we simplified the safety precondition of `unsafe_impl!`, but
did not remove safety proofs at call sites made redundant by that
simplification. This commit removes those now-obsolete proofs.

gherrit-pr-id: I70d5aa5ace6bd2e39e679eac7f00a66d4b843d57
github-merge-queue bot pushed a commit that referenced this pull request Mar 8, 2025
* Implement traits for Cell

Closes #1253

gherrit-pr-id: I569b74086a5f98cda71b4a4131f9ce4f89dcc623

* Remove obsolete safety proofs

In #2408, we simplified the safety precondition of `unsafe_impl!`, but
did not remove safety proofs at call sites made redundant by that
simplification. This commit removes those now-obsolete proofs.

gherrit-pr-id: I70d5aa5ace6bd2e39e679eac7f00a66d4b843d57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants