Skip to content
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

rework CastPtr, CastConstPtr, BoxCastPtr, ArcCastPtr #353

Merged
merged 2 commits into from
Oct 17, 2023

Conversation

cpu
Copy link
Member

@cpu cpu commented Oct 13, 2023

rework CastPtr, CastConstPtr, BoxCastPtr, ArcCastPtr

This commit reworks the existing CastPtr, CastConstPtr, BoxCastPtr, and ArcCastPtr traits into a new Castable trait with an associated Ownership, constrained to be a type implementing the new OwnershipMarker trait. Three implementations of this OwnershipMarker trait are offered: OwnershipBox, OwnershipArc and OwnershipRef.

The net result is that the type system is now able to enforce our invariant that a single Castable type can't be cast to a pointer of more than one type of ownership (e.g. both an Arc and a Box). Implementing Castable for the same type with different Ownership's or RustType's will be rejected by the compiler as a conflicting trait implementation. We always implement a distinct type per-ownership model, and per rust type.

Along the way crate-internal documentation for the traits and associated free-standing helper fss were reworked for clarity/consistency.

Resolves #349

lib: remove pub export of macros

Previously the helper macros in src/lib.rs were marked macro_export, making them part of the public API. Since these were
meant to be crate internal, we also annotated the macros as doc(hidden) to avoid them appearing in the API docs. I suspect this was done before pub(crate) visibility was an option.

This commit removes the macro_export and doc(hidden) attributes and uses a pub(crate) re-export to make the macros available to crate-internal users without making them part of the public API.

This also uncovered that the try_mut_slice! macro wasn't being used anywhere and so it is removed outright.

src/lib.rs Outdated Show resolved Hide resolved
src/cipher.rs Show resolved Hide resolved
src/connection.rs Show resolved Hide resolved
@cpu cpu force-pushed the cpu-349-safer-pointer-traits branch from fbeec7f to b13c99d Compare October 13, 2023 20:30
@jsha
Copy link
Collaborator

jsha commented Oct 13, 2023 via email

This commit reworks the existing `CastPtr`, `CastConstPtr`,
`BoxCastPtr`, and `ArcCastPtr` traits into a new `Castable` trait with
an associated `Ownership`, constrained to be a type implementing the new
`OwnershipMarker` trait. Three implementations of this `OwnershipMarker`
trait are offered: `OwnershipBox`, `OwnershipArc` and `OwnershipRef`.

The net result is that the type system is now able to enforce our
invariant that a single `Castable` type can't be cast to a pointer of
more than one type of ownership (e.g. both an `Arc` and a `Box`). We
always implement a distinct type per-ownership model.

Along the way crate-internal documentation for the traits and associated
free-standing helper `fs`s were reworked for clarity/consistency.
Previously the helper macros in `src/lib.rs` were marked
`macro_export`, making them part of the public API. Since these were
meant to be crate internal, we also annotated the macros as
`doc(hidden)` to avoid them appearing in the API docs. I suspect this
was done before `pub(crate)` visibility was an option.

This commit removes the `macro_export` and `doc(hidden)` attributes and
uses a `pub(crate)` re-export to make the macros available to
crate-internal users without making them part of the public API.

Along the way this uncovered that the `try_mut_slice!` macro wasn't
being used anywhere and so it is removed outright.
@cpu cpu force-pushed the cpu-349-safer-pointer-traits branch from b13c99d to f86f39c Compare October 13, 2023 20:40
Copy link
Collaborator

@jsha jsha left a comment

Choose a reason for hiding this comment

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

This is a great improvement, thanks!

@jsha jsha merged commit 14908f4 into rustls:main Oct 17, 2023
21 checks passed
@cpu
Copy link
Member Author

cpu commented Oct 17, 2023

Thanks for all your help along the way 🙇

@cpu cpu deleted the cpu-349-safer-pointer-traits branch October 17, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent implementing both ArcCastPtr and BoxCastPtr on the same type
2 participants