arc_castable!, box_castable!, ref_castable! macros #404
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cpu
commented
Mar 30, 2024
| /// | ||
| /// If a lifetime is specified, the opaque struct will be parameterized by that lifetime | ||
| /// and a `PhantomData` field referencing the lifetime is added to the struct. | ||
| macro_rules! ref_castable { |
There was a problem hiding this comment.
I made an attempt to fold the implementation of ref_castable! into castable! so the bulk of the macro body could be shared like box_castable! and arc_castable! but I wasn't happy with the result and so went back to having this macro live on its own. I've only written a handful of simple macros so its possible someone with more experience could refactor this out.
jsha
reviewed
Apr 18, 2024
jsha
reviewed
Apr 18, 2024
cpu
force-pushed
the
cpu-castable-macros
branch
2 times, most recently
from
dd01a1c to
7da8ff9
Compare
jsha
approved these changes
Apr 19, 2024
`try_arc_from` became `clone_arc`.
Throughout the project we follow a pattern of: 1. Creating a `snake_case_named` opaque struct[0]. 2. Implementing `Castable` for the struct: a. specifying a `RustType` (some native Rust type). b. specifying an `Ownership` (`OwnershipBox`, `OwnershipArc`, `OwnershipRef`). In the case of `OwnershipRef` we may also need to add a `PhantomData` field referencing a lifetime to the opaque struct. To reduce the boilerplate of doing all of the above over and over this commit adds three new macros that accomplish the task with less ceremony: `arc_castable!`, `box_castable!`, and `ref_castable!`. Existing instances of the pattern are updated to use these macros with one exception: the `rustls_client_hello` type implements `Castable` with `OwnershipRef` for `rustls_client_hello<'a>`, but it isn't the same opaque struct pattern as used elsewhere and so is best implemented by hand. [0]: https://github.com/rustls/rustls-ffi/blob/main/CONTRIBUTING.md#opaque-struct-pattern
cpu
force-pushed
the
cpu-castable-macros
branch
from
7da8ff9 to
33d279f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Throughout the project we follow a pattern of implementing safe bridge types for FFI by:
snake_case_namedopaque struct.Castablefor the struct:a. specifying a
RustType(some native Rust type).b. specifying an
Ownership(OwnershipBox,OwnershipArc,OwnershipRef).In the case of
OwnershipRefwe may also need to add aPhantomDatafield referencing a lifetime to the opaque struct.To reduce the boilerplate of doing all of the above over and over this commit adds three new macros (loosely inspired by the Rustls
enum_builder!macro) that accomplish the task with less ceremony (as suggested in #151):arc_castable!,box_castable!, andref_castable!.Care has been taken to avoid the downside mentioned in 151 about rustdoc comments. The macros ensure the doc comments we add are associated with the correct type and not the macro itself. We can verify this fact by noting that the generated
rustls.hheader file that contains these comments has not changed with the introduction of the macros.Existing instances of the pattern are updated to use these macros with one exception: the
rustls_client_hellotype implementsCastablewithOwnershipRefforrustls_client_hello<'a>, but it isn't the same opaque struct pattern as used elsewhere and so is best implemented by hand.I've spot checked the expansion of the macros against my expectations and was happy with the results. If you want to do the same thing, here's a
cargo expandof 10c5324.Resolves #151