Skip to content

Commit

Permalink
Remove StableDeref bound on Option<Cart> methods (#4457)
Browse files Browse the repository at this point in the history
We don't need it
  • Loading branch information
Manishearth authored Dec 13, 2023
1 parent 0b51dc1 commit 94fa985
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
- Correctly handle chars (https://github.com/unicode-org/icu4x/pull/4349)
- JS
- Fixed a bug where slice length is computed incorrectly (https://github.com/rust-diplomat/diplomat/pull/372)
- Utilities
- `yoke`
- Remove `StableDeref` bound from `Yoke<Y, Option<C>>` methods (https://github.com/unicode-org/icu4x/pull/4457)

## icu4x 1.4 (Nov 16, 2023)

Expand Down
11 changes: 10 additions & 1 deletion utils/yoke/src/yoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ pub struct Yoke<Y: for<'a> Yokeable<'a>, C> {
// must be the first field for drop order
// this will have a 'static lifetime parameter, that parameter is a lie
yokeable: KindaSortaDangling<Y>,
// Safety invariant: this type can be anything, but `yokeable` may only contain references to
// StableDeref parts of this cart, and those references must be valid for the lifetime of
// this cart (it must own or borrow them). It's ok for this cart to contain stack data as long as it
// is not referenced by `yokeable` during construction. `attach_to_cart`, the typical constructor
// of this type, upholds this invariant, but other constructors like `replace_cart` need to uphold it.
cart: C,
}

Expand Down Expand Up @@ -468,7 +473,9 @@ impl<Y: for<'a> Yokeable<'a>> Yoke<Y, ()> {
}
}

impl<Y: for<'a> Yokeable<'a>, C: StableDeref> Yoke<Y, Option<C>> {
// C does not need to be StableDeref here, if the yoke was constructed it's valid,
// and new_owned() doesn't construct a yokeable that uses references,
impl<Y: for<'a> Yokeable<'a>, C> Yoke<Y, Option<C>> {
/// Construct a new [`Yoke`] from static data. There will be no
/// references to `cart` here since [`Yokeable`]s are `'static`,
/// this is good for e.g. constructing fully owned
Expand Down Expand Up @@ -505,6 +512,8 @@ impl<Y: for<'a> Yokeable<'a>, C: StableDeref> Yoke<Y, Option<C>> {
/// If the cart is `None`, this returns `Some`, but if the cart is `Some`,
/// this returns `self` as an error.
pub fn try_into_yokeable(self) -> Result<Y, Self> {
// Safety: if the cart is None there is no way for the yokeable to
// have references into it because of the cart invariant.
match self.cart {
Some(_) => Err(self),
None => Ok(self.yokeable.into_inner()),
Expand Down

0 comments on commit 94fa985

Please sign in to comment.