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

[ptr] Add TransparentWrapper #958

Merged
merged 1 commit into from
Feb 28, 2024
Merged

[ptr] Add TransparentWrapper #958

merged 1 commit into from
Feb 28, 2024

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Feb 27, 2024

No description provided.

@joshlf joshlf force-pushed the transparent-wrapper branch 3 times, most recently from 7602838 to edf0f78 Compare February 27, 2024 21:12
@joshlf joshlf changed the title [WIP][ptr] Add TransparentWrapper [ptr] Add TransparentWrapper Feb 27, 2024
@joshlf joshlf requested a review from jswrenn February 27, 2024 21:12
@joshlf joshlf marked this pull request as ready for review February 27, 2024 21:12
TransparentWrapper is implemented for wrapper types which transparently
wrap other types (with the same layout and `UnsafeCell` ranges). This
allows us to express a number of conversions more generically, and write
less location-specific `unsafe` code.
Comment on lines +198 to +209
// SAFETY: `Wrapping<T>` has only one field, which is `pub` [2]. We are also
// guaranteed per [1] (from the comment above) that `Wrapping<T>` has the
// same layout as `T`. The only way for both of these to be true
// simultaneously is for `Wrapping<T>` to have the same bit validity as `T`.
// In particular, in order to change the bit validity, one of the following
// would need to happen:
// - `Wrapping` could change its `repr`, but this would violate the layout
// guarantee.
// - `Wrapping` could add or change its fields, but this would be a
// stability-breaking change.
//
// [2] https://doc.rust-lang.org/core/num/struct.Wrapping.html
Copy link
Member Author

Choose a reason for hiding this comment

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

@jswrenn Following up from our conversation offline - you had more suggestions for this comment?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine,

Comment on lines +198 to +209
// SAFETY: `Wrapping<T>` has only one field, which is `pub` [2]. We are also
// guaranteed per [1] (from the comment above) that `Wrapping<T>` has the
// same layout as `T`. The only way for both of these to be true
// simultaneously is for `Wrapping<T>` to have the same bit validity as `T`.
// In particular, in order to change the bit validity, one of the following
// would need to happen:
// - `Wrapping` could change its `repr`, but this would violate the layout
// guarantee.
// - `Wrapping` could add or change its fields, but this would be a
// stability-breaking change.
//
// [2] https://doc.rust-lang.org/core/num/struct.Wrapping.html
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine,

@joshlf joshlf added this pull request to the merge queue Feb 28, 2024
Merged via the queue into main with commit f857fbc Feb 28, 2024
210 checks passed
@joshlf joshlf deleted the transparent-wrapper branch February 28, 2024 21:33
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.

None yet

2 participants