Yoke::wrap_cart_in_box would have UB if Box::new panics #7431
Description
As far as I'm aware, Box::new is permitted to panic (see handle_alloc_error). Therefore, Yoke::wrap_cart_in_box can violate the safety contract of Yoke::replace_with_cart.
Possible Solution
Instead of the current implementation:
pub unsafe fn replace_cart<C2>(self, f: impl FnOnce(C) -> C2) -> Yoke<Y, C2> {
Yoke {
// Safety note: the safety invariant of this function guarantees that
// the data that the yokeable references has its ownership (if any)
// transferred to the new cart before self.cart is dropped.
yokeable: self.yokeable,
cart: f(self.cart),
}
}I think it would suffice to do:
pub unsafe fn replace_cart<C2>(self, f: impl FnOnce(C) -> C2) -> Yoke<Y, C2> {
let yokeable = ManuallyDrop::new(self.yokeable);
let cart = f(self.cart);
Yoke {
// Safety note: the safety invariant of this function guarantees that
// the data that the yokeable references has its ownership (if any)
// transferred to the new cart before self.cart is dropped, unless
// `f` panics, in which case the above `ManuallyDrop` ensures that
// the yokeable is leaked (preventing any UB from dropping the
// yokeable after its cart).
yokeable: yokeable.into_inner(),
cart,
}
}The safety docs would also have to be updated (this strictly loosens the restrictions placed on users, so it should not be breaking), and it should instead be noted that if f panics, then the yokeable is leaked (and the panic is propagated).