Skip to content

Commit

Permalink
Fix ReadRef<T>::cell when T != T::Read::Repr
Browse files Browse the repository at this point in the history
  • Loading branch information
bgw committed Jul 29, 2024
1 parent 12f7f73 commit 417aec9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 13 deletions.
1 change: 0 additions & 1 deletion crates/turbo-tasks-memory/tests/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ async fn test_changing_generic() {

// Test that we can convert a `Vc` to a `ReadRef`, and then back to a `Vc`.
#[tokio::test]
#[ignore = "TODO: This panics! There's a casting bug in the generics implementation."]
async fn test_read_ref_round_trip() {
run(&REGISTRATION, async move {
let c: Vc<Option<Vc<u8>>> = Vc::cell(Some(Vc::cell(1)));
Expand Down
10 changes: 8 additions & 2 deletions crates/turbo-tasks/src/read_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
debug::{ValueDebugFormat, ValueDebugFormatString},
macro_helpers::find_cell_by_type,
trace::{TraceRawVcs, TraceRawVcsContext},
triomphe_utils::unchecked_sidecast_triomphe_arc,
SharedReference, Vc, VcRead, VcValueType,
};

Expand Down Expand Up @@ -242,8 +243,13 @@ where
/// Returns a new cell that points to the same value as the given
/// reference.
pub fn cell(read_ref: ReadRef<T>) -> Vc<T> {
let local_cell = find_cell_by_type(T::get_value_type_id());
local_cell.update_shared_reference(SharedReference::new(read_ref.0));
let type_id = T::get_value_type_id();
let local_cell = find_cell_by_type(type_id);
// SAFETY: `T` and `T::Read::Repr` must have equivalent memory representations,
// guaranteed by the unsafe implementation of `VcValueType`.
local_cell.update_shared_reference(SharedReference::new(unsafe {
unchecked_sidecast_triomphe_arc::<T, <T::Read as VcRead<T>>::Repr>(read_ref.0)
}));
Vc {
node: local_cell.into(),
_t: PhantomData,
Expand Down
27 changes: 17 additions & 10 deletions crates/turbo-tasks/src/triomphe_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,28 @@ pub fn downcast_triomphe_arc<T: Any + Send + Sync>(
this: triomphe::Arc<dyn Any + Send + Sync>,
) -> Result<triomphe::Arc<T>, triomphe::Arc<dyn Any + Send + Sync>> {
if (*this).is::<T>() {
unsafe {
// Get the pointer to the offset (*const T) inside of the ArcInner.
let ptr = triomphe::Arc::into_raw(this);
// SAFETY: The negative offset from the data (ptr) in an Arc to the start of the
// data structure is fixed regardless of type `T`.
//
// SAFETY: Casting from a fat pointer to a thin pointer is safe, as long as the
// types are compatible (they are).
Ok(triomphe::Arc::from_raw(ptr.cast()))
}
unsafe { Ok(unchecked_sidecast_triomphe_arc(this)) }
} else {
Err(this)
}
}

pub unsafe fn unchecked_sidecast_triomphe_arc<T, U>(this: triomphe::Arc<T>) -> triomphe::Arc<U>
where
T: ?Sized,
{
unsafe {
// Get the pointer to the offset (*const T) inside of the ArcInner.
let ptr = triomphe::Arc::into_raw(this);
// SAFETY: The negative offset from the data (ptr) in an Arc to the start of the
// data structure is fixed regardless of type `T`.
//
// SAFETY: Casting from a fat pointer to a thin pointer is safe, as long as the
// types are compatible (they are).
triomphe::Arc::from_raw(ptr.cast())
}
}

/// [`Coercion::to_any`] except that it coerces to `dyn Any + Send + Sync` as
/// opposed to `dyn Any`.
pub fn coerce_to_any_send_sync<T: Any + Send + Sync>() -> Coercion<T, dyn Any + Send + Sync> {
Expand Down

0 comments on commit 417aec9

Please sign in to comment.