Add a minimum viable ResolvedValue marker trait (vercel/turborepo#8662)

## Description This trait will provide a best-effort indication that a type is safe to return from a local task function. This PR also makes some improvements to the `TraceRawVcs` impls that I noticed as opportunities while working on `ResolvedValue`. ## Why? https://www.notion.so/vercel/Resolved-Vcs-Vc-Lifetimes-Local-Vcs-and-Vc-Refcounts-49d666d3f9594017b5b312b87ddc5bff ## Testing Instructions Tested as part of vercel/turborepo#8678
vercel · Jul 9, 2024 · a6029e9 · a6029e9
1 parent ac9cb1f
commit a6029e9
Show file tree

Hide file tree

Showing 4 changed files with 147 additions and 28 deletions.
diff --git a/crates/turbo-tasks/src/lib.rs b/crates/turbo-tasks/src/lib.rs
@@ -107,9 +107,9 @@ pub use turbo_tasks_macros::{function, value, value_impl, value_trait, TaskInput
 pub use value::{TransientInstance, TransientValue, Value};
 pub use value_type::{TraitMethod, TraitType, ValueType};
 pub use vc::{
- Dynamic, ResolvedVc, TypedForInput, Upcast, ValueDefault, Vc, VcCast, VcCellNewMode,
- VcCellSharedMode, VcDefaultRead, VcRead, VcTransparentRead, VcValueTrait, VcValueTraitCast,
- VcValueType, VcValueTypeCast,
+ Dynamic, ResolvedValue, ResolvedVc, TypedForInput, Upcast, ValueDefault, Vc, VcCast,
+ VcCellNewMode, VcCellSharedMode, VcDefaultRead, VcRead, VcTransparentRead, VcValueTrait,
+ VcValueTraitCast, VcValueType, VcValueTypeCast,
 };
 
 pub use crate::rcstr::RcStr;

diff --git a/crates/turbo-tasks/src/trace.rs b/crates/turbo-tasks/src/trace.rs
@@ -1,6 +1,7 @@
 use std::{
  cell::RefCell,
  collections::{BTreeMap, BTreeSet, HashMap, HashSet},
+ marker::PhantomData,
  path::{Path, PathBuf},
  sync::{atomic::*, Arc, Mutex},
  time::Duration,
@@ -70,44 +71,55 @@ ignore!(
  AtomicBool,
  AtomicUsize
 );
-ignore!((), String, Duration, anyhow::Error, RcStr);
+ignore!((), str, String, Duration, anyhow::Error, RcStr);
 ignore!(Path, PathBuf);
 ignore!(serde_json::Value);
 
-impl<'a> TraceRawVcs for &'a str {
+impl<T: ?Sized> TraceRawVcs for PhantomData<T> {
  fn trace_raw_vcs(&self, _trace_context: &mut TraceRawVcsContext) {}
 }
 
-impl<A: TraceRawVcs> TraceRawVcs for (A,) {
- fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
- TraceRawVcs::trace_raw_vcs(&self.0, trace_context);
- }
+// based on stdlib's internal `tuple_impls!` macro
+macro_rules! impl_trace_tuple {
+ ($T:ident) => {
+ impl_trace_tuple!(@impl $T);
+ };
+ ($T:ident $( $U:ident )+) => {
+ impl_trace_tuple!($( $U )+);
+ impl_trace_tuple!(@impl $T $( $U )+);
+ };
+ (@impl $( $T:ident )+) => {
+ impl<$($T: TraceRawVcs),+> TraceRawVcs for ($($T,)+) {
+ #[allow(non_snake_case)]
+ fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
+ let ($($T,)+) = self;
+ $(
+ TraceRawVcs::trace_raw_vcs($T, trace_context);
+ )+
+ }
+ }
+ };
 }
 
-impl<A: TraceRawVcs, B: TraceRawVcs> TraceRawVcs for (A, B) {
- fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
- TraceRawVcs::trace_raw_vcs(&self.0, trace_context);
- TraceRawVcs::trace_raw_vcs(&self.1, trace_context);
- }
-}
+impl_trace_tuple!(E D C B A Z Y X W V U T);
 
-impl<A: TraceRawVcs, B: TraceRawVcs, C: TraceRawVcs> TraceRawVcs for (A, B, C) {
+impl<T: TraceRawVcs> TraceRawVcs for Option<T> {
  fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
- TraceRawVcs::trace_raw_vcs(&self.0, trace_context);
- TraceRawVcs::trace_raw_vcs(&self.1, trace_context);
- TraceRawVcs::trace_raw_vcs(&self.2, trace_context);
+ if let Some(item) = self {
+  TraceRawVcs::trace_raw_vcs(item, trace_context);
+ }
  }
 }
 
-impl<T: TraceRawVcs> TraceRawVcs for Option<T> {
+impl<T: TraceRawVcs> TraceRawVcs for Vec<T> {
  fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
- if let Some(item) = self {
+ for item in self.iter() {
  TraceRawVcs::trace_raw_vcs(item, trace_context);
  }
  }
 }
 
-impl<T: TraceRawVcs> TraceRawVcs for Vec<T> {
+impl<T: TraceRawVcs, const N: usize> TraceRawVcs for [T; N] {
  fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
  for item in self.iter() {
  TraceRawVcs::trace_raw_vcs(item, trace_context);
@@ -123,7 +135,7 @@ impl<T: TraceRawVcs, S> TraceRawVcs for HashSet<T, S> {
  }
 }
 
-impl<T: TraceRawVcs, S> TraceRawVcs for AutoSet<T, S> {
+impl<T: TraceRawVcs, S, const I: usize> TraceRawVcs for AutoSet<T, S, I> {
  fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
  for item in self.iter() {
  TraceRawVcs::trace_raw_vcs(item, trace_context);
@@ -156,7 +168,7 @@ impl<K: TraceRawVcs, V: TraceRawVcs, S> TraceRawVcs for HashMap<K, V, S> {
  }
 }
 
-impl<K: TraceRawVcs, V: TraceRawVcs, S> TraceRawVcs for AutoMap<K, V, S> {
+impl<K: TraceRawVcs, V: TraceRawVcs, S, const I: usize> TraceRawVcs for AutoMap<K, V, S, I> {
  fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
  for (key, value) in self.iter() {
  TraceRawVcs::trace_raw_vcs(key, trace_context);
@@ -210,16 +222,27 @@ impl<T: TraceRawVcs, E: TraceRawVcs> TraceRawVcs for Result<T, E> {
  }
 }
 
-impl<T: TraceRawVcs> TraceRawVcs for Mutex<T> {
+impl<T: TraceRawVcs + ?Sized> TraceRawVcs for Mutex<T> {
  fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
  self.lock().unwrap().trace_raw_vcs(trace_context);
  }
 }
 
-impl<T: TraceRawVcs> TraceRawVcs for RefCell<T> {
+impl<T: TraceRawVcs + ?Sized> TraceRawVcs for RefCell<T> {
  fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
  self.borrow().trace_raw_vcs(trace_context);
  }
 }
 
+impl<T: TraceRawVcs + ?Sized> TraceRawVcs for &T {
+ fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
+ (**self).trace_raw_vcs(trace_context);
+ }
+}
+impl<T: TraceRawVcs + ?Sized> TraceRawVcs for &mut T {
+ fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
+ (**self).trace_raw_vcs(trace_context);
+ }
+}
+
 pub use turbo_tasks_macros::TraceRawVcs;
diff --git a/crates/turbo-tasks/src/vc/mod.rs b/crates/turbo-tasks/src/vc/mod.rs
@@ -22,7 +22,7 @@ pub use self::{
  cell_mode::{VcCellNewMode, VcCellSharedMode},
  default::ValueDefault,
  read::{ReadVcFuture, VcDefaultRead, VcRead, VcTransparentRead},
- resolved::ResolvedVc,
+ resolved::{ResolvedValue, ResolvedVc},
  traits::{Dynamic, TypedForInput, Upcast, VcValueTrait, VcValueType},
 };
 use crate::{

diff --git a/crates/turbo-tasks/src/vc/resolved.rs b/crates/turbo-tasks/src/vc/resolved.rs
@@ -1,9 +1,24 @@
 use std::{
+ cell::RefCell,
+ collections::{BTreeMap, BTreeSet, HashMap, HashSet},
  hash::{Hash, Hasher},
+ marker::PhantomData,
  ops::Deref,
+ path::{Path, PathBuf},
+ sync::{
+ atomic::{
+ AtomicBool, AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicU16, AtomicU32, AtomicU64,
+ AtomicU8, AtomicUsize,
+ },
+ Arc, Mutex,
+ },
+ time::Duration,
 };
 
-use crate::vc::Vc;
+use auto_hash_map::{AutoMap, AutoSet};
+use indexmap::{IndexMap, IndexSet};
+
+use crate::{vc::Vc, RcStr};
 
 #[derive(Copy, Clone)]
 pub struct ResolvedVc<T>
@@ -32,3 +47,84 @@ where
  self.node.hash(state);
  }
 }
+
+/// Indicates that a type does not contain any instances of [`Vc`]. It may
+/// contain [`ResolvedVc`].
+///
+/// # Safety
+///
+/// This trait is marked as unsafe. You should not derive it yourself, but
+/// instead you should rely on [`#[turbo_tasks::value(resolved)]`][macro@
+/// turbo_tasks::value] to do it for you.
+pub unsafe trait ResolvedValue {}
+
+unsafe impl<T: ?Sized + Send + ResolvedValue> ResolvedValue for ResolvedVc<T> {}
+
+macro_rules! impl_resolved {
+ ($ty:ty) => {
+ unsafe impl ResolvedValue for $ty {}
+ };
+
+ ($ty:ty, $($tys:ty),+) => {
+ impl_resolved!($ty);
+ impl_resolved!($($tys),+);
+ }
+}
+
+impl_resolved!(i8, u8, i16, u16, i32, u32, i64, u64, f32, f64, char, bool, usize);
+impl_resolved!(
+ AtomicI8,
+ AtomicU8,
+ AtomicI16,
+ AtomicU16,
+ AtomicI32,
+ AtomicU32,
+ AtomicI64,
+ AtomicU64,
+ AtomicBool,
+ AtomicUsize
+);
+impl_resolved!((), str, String, Duration, anyhow::Error, RcStr);
+impl_resolved!(Path, PathBuf);
+impl_resolved!(serde_json::Value);
+
+// based on stdlib's internal `tuple_impls!` macro
+macro_rules! impl_resolved_tuple {
+ ($T:ident) => {
+ impl_resolved_tuple!(@impl $T);
+ };
+ ($T:ident $( $U:ident )+) => {
+ impl_resolved_tuple!($( $U )+);
+ impl_resolved_tuple!(@impl $T $( $U )+);
+ };
+ (@impl $( $T:ident )+) => {
+ unsafe impl<$($T: ResolvedValue),+> ResolvedValue for ($($T,)+) {}
+ };
+}
+
+impl_resolved_tuple!(E D C B A Z Y X W V U T);
+
+unsafe impl<T: ResolvedValue> ResolvedValue for Option<T> {}
+unsafe impl<T: ResolvedValue> ResolvedValue for Vec<T> {}
+unsafe impl<T: ResolvedValue, const N: usize> ResolvedValue for [T; N] {}
+unsafe impl<T: ResolvedValue> ResolvedValue for [T] {}
+unsafe impl<T: ResolvedValue, S> ResolvedValue for HashSet<T, S> {}
+unsafe impl<T: ResolvedValue, S, const I: usize> ResolvedValue for AutoSet<T, S, I> {}
+unsafe impl<T: ResolvedValue> ResolvedValue for BTreeSet<T> {}
+unsafe impl<T: ResolvedValue, S> ResolvedValue for IndexSet<T, S> {}
+unsafe impl<K: ResolvedValue, V: ResolvedValue, S> ResolvedValue for HashMap<K, V, S> {}
+unsafe impl<K: ResolvedValue, V: ResolvedValue, S, const I: usize> ResolvedValue
+ for AutoMap<K, V, S, I>
+{
+}
+unsafe impl<K: ResolvedValue, V: ResolvedValue> ResolvedValue for BTreeMap<K, V> {}
+unsafe impl<K: ResolvedValue, V: ResolvedValue, S> ResolvedValue for IndexMap<K, V, S> {}
+unsafe impl<T: ResolvedValue + ?Sized> ResolvedValue for Box<T> {}
+unsafe impl<T: ResolvedValue + ?Sized> ResolvedValue for Arc<T> {}
+unsafe impl<T: ResolvedValue, E: ResolvedValue> ResolvedValue for Result<T, E> {}
+unsafe impl<T: ResolvedValue + ?Sized> ResolvedValue for Mutex<T> {}
+unsafe impl<T: ResolvedValue + ?Sized> ResolvedValue for RefCell<T> {}
+unsafe impl<T: ?Sized> ResolvedValue for PhantomData<T> {}
+
+unsafe impl<T: ResolvedValue + ?Sized> ResolvedValue for &T {}
+unsafe impl<T: ResolvedValue + ?Sized> ResolvedValue for &mut T {}