refactor(turbo-tasks): Remove task id from RawVc::LocalOutput

bgw · bgw · commit 026c5f8eb432 · 2025-04-25T14:13:24.000-07:00
diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs
@@ -1124,11 +1124,16 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
             let task_name = task_type.get_name();
 
             let cause_self = task_type.this.and_then(|cause_self_raw_vc| {
-                let task_id = cause_self_raw_vc.get_task_id();
+                let Some(task_id) = cause_self_raw_vc.try_get_task_id() else {
+                    // `task_id` should never be `None` at this point, as that would imply a
+                    // non-local task is returning a local `Vc`...
+                    // Just ignore if it happens, as we're likely already panicking.
+                    return None;
+                };
                 if task_id.is_transient() {
                     Some(Box::new(inner_id(
                         backend,
-                        cause_self_raw_vc.get_task_id(),
+                        task_id,
                         cause_self_raw_vc.try_get_type_id(),
                         visited_set,
                     )))
@@ -1140,8 +1145,16 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
                 .arg
                 .get_raw_vcs()
                 .into_iter()
-                .map(|raw_vc| (raw_vc.get_task_id(), raw_vc.try_get_type_id()))
-                .filter(|(task_id, _)| task_id.is_transient())
+                .filter_map(|raw_vc| {
+                    let Some(task_id) = raw_vc.try_get_task_id() else {
+                        // `task_id` should never be `None` (see comment above)
+                        return None;
+                    };
+                    if task_id.is_transient() {
+                        return None;
+                    }
+                    Some((task_id, raw_vc.try_get_type_id()))
+                })
                 .collect::<IndexSet<_>>() // dedupe
                 .into_iter()
                 .map(|(task_id, cell_type_id)| {
@@ -2184,24 +2197,25 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
         // these checks occur in a potentially hot codepath, but they're cheap
         let RawVc::TaskCell(col_task_id, col_cell_id) = collectible else {
             // This should never happen: The collectible APIs use ResolvedVc
-            let task_info =
-                if let Some(col_task_ty) = self.lookup_task_type(collectible.get_task_id()) {
-                    Cow::Owned(format!(" (return type of {})", col_task_ty))
-                } else {
-                    Cow::Borrowed("")
-                };
+            let task_info = if let Some(col_task_ty) = collectible
+                .try_get_task_id()
+                .and_then(|t| self.lookup_task_type(t))
+            {
+                Cow::Owned(format!(" (return type of {col_task_ty})"))
+            } else {
+                Cow::Borrowed("")
+            };
             panic!("Collectible{task_info} must be a ResolvedVc")
         };
         if col_task_id.is_transient() && !task_id.is_transient() {
-            let transient_reason =
-                if let Some(col_task_ty) = self.lookup_task_type(collectible.get_task_id()) {
-                    Cow::Owned(format!(
-                        ". The collectible is transient because it depends on:\n{}",
-                        self.debug_trace_transient_task(&col_task_ty, Some(col_cell_id)),
-                    ))
-                } else {
-                    Cow::Borrowed("")
-                };
+            let transient_reason = if let Some(col_task_ty) = self.lookup_task_type(col_task_id) {
+                Cow::Owned(format!(
+                    ". The collectible is transient because it depends on:\n{}",
+                    self.debug_trace_transient_task(&col_task_ty, Some(col_cell_id)),
+                ))
+            } else {
+                Cow::Borrowed("")
+            };
             // this should never happen: How would a persistent function get a transient Vc?
             panic!(
                 "Collectible is transient, transient collectibles cannot be emitted from \
diff --git a/turbopack/crates/turbo-tasks/src/manager.rs b/turbopack/crates/turbo-tasks/src/manager.rs
@@ -48,6 +48,8 @@ use crate::{
     VcValueType,
 };
 
+/// Common base trait for [`TurboTasksApi`] and [`TurboTasksBackendApi`]. Provides APIs for creating
+/// tasks from function calls.
 pub trait TurboTasksCallApi: Sync + Send {
     /// Calls a native function with arguments. Resolves arguments when needed
     /// with a wrapper task.
@@ -93,6 +95,11 @@ pub trait TurboTasksCallApi: Sync + Send {
     ) -> TaskId;
 }
 
+/// A type-erased subset of [`TurboTasks`] stored inside a thread local when we're in a turbo task
+/// context. Returned by the [`turbo_tasks`] helper function.
+///
+/// This trait is needed because thread locals cannot contain an unresolved [`Backend`] type
+/// parameter.
 pub trait TurboTasksApi: TurboTasksCallApi + Sync + Send {
     fn pin(&self) -> Arc<dyn TurboTasksApi>;
 
@@ -135,9 +142,20 @@ pub trait TurboTasksApi: TurboTasksCallApi + Sync + Send {
         options: ReadCellOptions,
     ) -> Result<Result<TypedCellContent, EventListener>>;
 
+    /// Reads a [`RawVc::LocalOutput`]. If the task has completed, returns the [`RawVc`] the local
+    /// task points to.
+    ///
+    /// The returned [`RawVc`] may also be a [`RawVc::LocalOutput`], so this may need to be called
+    /// recursively or in a loop.
+    ///
     /// This does not accept a consistency argument, as you cannot control consistency of a read of
     /// an operation owned by your own task. Strongly consistent reads are only allowed on
-    /// `OperationVc`s, which should never be local tasks.
+    /// [`OperationVc`]s, which should never be local tasks.
+    ///
+    /// No dependency tracking will happen as a result of this function call, as it's a no-op for a
+    /// task to depend on itself.
+    ///
+    /// [`OperationVc`]: crate::OperationVc
     fn try_read_local_output(
         &self,
         execution_id: ExecutionId,
@@ -216,6 +234,7 @@ impl<T> Unused<T> {
     }
 }
 
+/// A subset of the [`TurboTasks`] API that's exposed to [`Backend`] implementations.
 pub trait TurboTasksBackendApi<B: Backend + 'static>: TurboTasksCallApi + Sync + Send {
     fn pin(&self) -> Arc<dyn TurboTasksBackendApi<B>>;
 
@@ -265,8 +284,8 @@ pub trait TurboTasksBackendApi<B: Backend + 'static>: TurboTasksCallApi + Sync +
     fn backend(&self) -> &B;
 }
 
-/// An extension trait for methods of `TurboTasksBackendApi` that are not object-safe. This is
-/// automatically implemented for all `TurboTasksBackendApi`s using a blanket impl.
+/// An extension trait for methods of [`TurboTasksBackendApi`] that are not object-safe. This is
+/// automatically implemented for all [`TurboTasksBackendApi`]s using a blanket impl.
 pub trait TurboTasksBackendApiExt<B: Backend + 'static>: TurboTasksBackendApi<B> {
     /// Allows modification of the [`Backend::TaskState`].
     ///
@@ -768,6 +787,8 @@ impl<B: Backend + 'static> TurboTasks<B> {
             self.backend.get_task_description(parent_task_id),
             ty,
         );
+        #[cfg(not(feature = "tokio_tracing"))]
+        let _ = parent_task_id; // suppress unused variable warning
 
         let this = self.pin();
         let future = async move {
@@ -822,7 +843,7 @@ impl<B: Backend + 'static> TurboTasks<B> {
         #[cfg(not(feature = "tokio_tracing"))]
         tokio::task::spawn(future);
 
-        RawVc::LocalOutput(parent_task_id, persistence, execution_id, local_task_id)
+        RawVc::LocalOutput(persistence, execution_id, local_task_id)
     }
 
     fn begin_primary_job(&self) {
diff --git a/turbopack/crates/turbo-tasks/src/raw_vc.rs b/turbopack/crates/turbo-tasks/src/raw_vc.rs
@@ -57,7 +57,7 @@ impl Display for CellId {
 pub enum RawVc {
     TaskOutput(TaskId),
     TaskCell(TaskId, CellId),
-    LocalOutput(TaskId, TaskPersistence, ExecutionId, LocalTaskId),
+    LocalOutput(TaskPersistence, ExecutionId, LocalTaskId),
 }
 
 impl RawVc {
@@ -84,7 +84,7 @@ impl RawVc {
     pub fn is_transient(&self) -> bool {
         match self {
             RawVc::TaskOutput(task) | RawVc::TaskCell(task, ..) => task.is_transient(),
-            RawVc::LocalOutput(_, persistence, ..) => *persistence == TaskPersistence::Transient,
+            RawVc::LocalOutput(persistence, ..) => *persistence == TaskPersistence::Transient,
         }
     }
 
@@ -148,7 +148,7 @@ impl RawVc {
                         return Err(ResolveTypeError::NoContent);
                     }
                 }
-                RawVc::LocalOutput(_task_id, _persistence, execution_id, local_task_id) => {
+                RawVc::LocalOutput(_persistence, execution_id, local_task_id) => {
                     current = read_local_output(&*tt, execution_id, local_task_id)
                         .await
                         .map_err(|source| ResolveTypeError::TaskError { source })?;
@@ -185,7 +185,7 @@ impl RawVc {
                     consistency = ReadConsistency::Eventual;
                 }
                 RawVc::TaskCell(_, _) => return Ok(current),
-                RawVc::LocalOutput(_task_id, _persistence, execution_id, local_task_id) => {
+                RawVc::LocalOutput(_persistence, execution_id, local_task_id) => {
                     debug_assert_eq!(consistency, ReadConsistency::Eventual);
                     current = read_local_output(&*tt, execution_id, local_task_id).await?;
                 }
@@ -200,7 +200,7 @@ impl RawVc {
         let mut current = self;
         loop {
             match current {
-                RawVc::LocalOutput(_task_id, _persistence, execution_id, local_task_id) => {
+                RawVc::LocalOutput(_persistence, execution_id, local_task_id) => {
                     current = read_local_output(&*tt, execution_id, local_task_id).await?;
                 }
                 non_local => return Ok(non_local),
@@ -209,13 +209,17 @@ impl RawVc {
     }
 
     pub(crate) fn connect(&self) {
+        let RawVc::TaskOutput(task_id) = self else {
+            panic!("RawVc::connect() must only be called on a RawVc::TaskOutput");
+        };
         let tt = turbo_tasks();
-        tt.connect_task(self.get_task_id());
+        tt.connect_task(*task_id);
     }
 
-    pub fn get_task_id(&self) -> TaskId {
+    pub fn try_get_task_id(&self) -> Option<TaskId> {
         match self {
-            RawVc::TaskOutput(t) | RawVc::TaskCell(t, ..) | RawVc::LocalOutput(t, ..) => *t,
+            RawVc::TaskOutput(t) | RawVc::TaskCell(t, ..) => Some(*t),
+            RawVc::LocalOutput(..) => None,
         }
     }
 
@@ -250,20 +254,30 @@ impl RawVc {
 /// This implementation of `CollectiblesSource` assumes that `self` is a `RawVc::TaskOutput`.
 impl CollectiblesSource for RawVc {
     fn peek_collectibles<T: VcValueTrait + ?Sized>(self) -> AutoSet<ResolvedVc<T>> {
-        debug_assert!(matches!(self, RawVc::TaskOutput(..)));
+        let RawVc::TaskOutput(task_id) = self else {
+            panic!(
+                "<RawVc as CollectiblesSource>::peek_collectibles() must only be called on a \
+                 RawVc::TaskOutput"
+            );
+        };
         let tt = turbo_tasks();
         tt.notify_scheduled_tasks();
-        let map = tt.read_task_collectibles(self.get_task_id(), T::get_trait_type_id());
+        let map = tt.read_task_collectibles(task_id, T::get_trait_type_id());
         map.into_iter()
             .filter_map(|(raw, count)| (count > 0).then_some(raw.try_into().unwrap()))
             .collect()
     }
 
     fn take_collectibles<T: VcValueTrait + ?Sized>(self) -> AutoSet<ResolvedVc<T>> {
-        debug_assert!(matches!(self, RawVc::TaskOutput(..)));
+        let RawVc::TaskOutput(task_id) = self else {
+            panic!(
+                "<RawVc as CollectiblesSource>::take_collectibles() must only be called on a \
+                 RawVc::TaskOutput"
+            );
+        };
         let tt = turbo_tasks();
         tt.notify_scheduled_tasks();
-        let map = tt.read_task_collectibles(self.get_task_id(), T::get_trait_type_id());
+        let map = tt.read_task_collectibles(task_id, T::get_trait_type_id());
         tt.unemit_collectibles(T::get_trait_type_id(), &map);
         map.into_iter()
             .filter_map(|(raw, count)| (count > 0).then_some(raw.try_into().unwrap()))
@@ -360,7 +374,7 @@ impl Future for ReadRawVcFuture {
                             Err(err) => return Poll::Ready(Err(err)),
                         }
                     }
-                    RawVc::LocalOutput(_task_id, _persistence, execution_id, local_output_id) => {
+                    RawVc::LocalOutput(_persistence, execution_id, local_output_id) => {
                         debug_assert_eq!(this.consistency, ReadConsistency::Eventual);
                         let read_result = tt.try_read_local_output(execution_id, local_output_id);
                         match read_result {
