Repository: vercel/next.js
Pull request: [turbopack] Remove the implementation of `TaskInput` from `AutoSet`

TaskInputs should have deterministic serialization and that isn't really possible with `AutoSet`

This was only used as part of the `CapturedIssues` api, so that is modified to wrap the autoset in a `cell` instead of an `Arc`.  Originally the `Arc` was used to make it possible to call `peek_issues` from top level `run_once` tasks.  So to fix that, I audited callsites and moved one call of `peek_issues` from `handle_issues` into the `IssueReporter`.

As part of the audit, I changed the API of `take_issues` to `drop_issues` since that is the only usecase for this function.



Closes PACK-5677

Changed files:
filename: crates/next-api/src/operation.rs
patch: @@ -38,7 +38,7 @@ async fn entrypoints_without_collectibles_operation(
 ) -> Result<Vc<Entrypoints>> {
     let _ = entrypoints.resolve_strongly_consistent().await?;
     let _ = entrypoints.take_collectibles::<Box<dyn Diagnostic>>();
-    let _ = entrypoints.take_issues();
+    entrypoints.drop_issues();
     let _ = get_effects(entrypoints).await?;
     Ok(entrypoints.connect())
 }

filename: crates/next-api/src/project.rs
patch: @@ -991,7 +991,7 @@ impl Project {
                 // In development mode, we need to to take and drop the issues, otherwise every
                 // route will report all issues.
                 let vc = module_graphs_op.resolve_strongly_consistent().await?;
-                let _ = module_graphs_op.take_issues();
+                module_graphs_op.drop_issues();
                 *vc
             };
 

filename: turbopack/crates/turbo-tasks/src/task/task_input.rs
patch: @@ -19,6 +19,9 @@ use crate::{
 
 /// Trait to implement in order for a type to be accepted as a
 /// [`#[turbo_tasks::function]`][crate::function] argument.
+///
+/// Serialization must be deterministic and compatible with `eq` comparisons.  If two `TaskInputs`
+/// compare equal they must also serialize to the same bytes.
 pub trait TaskInput: Send + Sync + Clone + Debug + PartialEq + Eq + Hash + TraceRawVcs {
     fn resolve_input(&self) -> impl Future<Output = Result<Self>> + Send + '_ {
         async { Ok(self.clone()) }
@@ -332,19 +335,6 @@ where
     }
 }
 
-impl<T> TaskInput for auto_hash_map::AutoSet<T>
-where
-    T: TaskInput,
-{
-    fn is_resolved(&self) -> bool {
-        self.iter().all(TaskInput::is_resolved)
-    }
-
-    fn is_transient(&self) -> bool {
-        self.iter().any(TaskInput::is_transient)
-    }
-}
-
 macro_rules! tuple_impls {
     ( $( $name:ident )+ ) => {
         impl<$($name: TaskInput),+> TaskInput for ($($name,)+)

filename: turbopack/crates/turbopack-cli-utils/src/issue.rs
patch: @@ -15,8 +15,8 @@ use turbo_rcstr::RcStr;
 use turbo_tasks::{RawVc, TransientInstance, TransientValue, Vc};
 use turbo_tasks_fs::{FileLinesContent, source_context::get_source_context};
 use turbopack_core::issue::{
-    CapturedIssues, IssueReporter, IssueSeverity, PlainIssue, PlainIssueSource, PlainTraceItem,
-    StyledString,
+    CollectibleIssuesExt, IssueReporter, IssueSeverity, PlainIssue, PlainIssueSource,
+    PlainTraceItem, StyledString,
 };
 
 use crate::source_context::format_source_context_lines;
@@ -357,11 +357,10 @@ impl IssueReporter for ConsoleUi {
     #[turbo_tasks::function]
     async fn report_issues(
         &self,
-        issues: TransientInstance<CapturedIssues>,
         source: TransientValue<RawVc>,
         min_failing_severity: IssueSeverity,
     ) -> Result<Vc<bool>> {
-        let issues = &*issues;
+        let issues = source.peek_issues();
         let LogOptions {
             ref current_dir,
             ref project_dir,

filename: turbopack/crates/turbopack-core/src/issue/mod.rs
patch: @@ -6,18 +6,16 @@ pub mod resolve;
 use std::{
     cmp::min,
     fmt::{Display, Formatter},
-    sync::Arc,
 };
 
 use anyhow::{Result, anyhow};
-use async_trait::async_trait;
 use auto_hash_map::AutoSet;
 use serde::{Deserialize, Serialize};
 use turbo_rcstr::RcStr;
 use turbo_tasks::{
     CollectiblesSource, IntoTraitRef, NonLocalValue, OperationVc, RawVc, ReadRef, ResolvedVc,
-    TaskInput, TransientInstance, TransientValue, TryJoinIterExt, Upcast, ValueDefault,
-    ValueToString, Vc, emit, trace::TraceRawVcs,
+    TaskInput, TransientValue, TryJoinIterExt, Upcast, ValueDefault, ValueToString, Vc, emit,
+    trace::TraceRawVcs,
 };
 use turbo_tasks_fs::{FileContent, FileLine, FileLinesContent, FileSystem, FileSystemPath};
 use turbo_tasks_hash::{DeterministicHash, Xxh3Hash64Hasher};
@@ -164,9 +162,8 @@ pub trait ImportTracer {
     fn get_traces(self: Vc<Self>, path: FileSystemPath) -> Vc<ImportTraces>;
 }
 
-#[derive(
-    Debug, Clone, TaskInput, TraceRawVcs, Hash, Eq, PartialEq, Serialize, Deserialize, NonLocalValue,
-)]
+#[turbo_tasks::value]
+#[derive(Debug)]
 pub struct DelegatingImportTracer {
     delegates: AutoSet<ResolvedVc<Box<dyn ImportTracer>>>,
 }
@@ -220,7 +217,7 @@ pub struct Issues(Vec<ResolvedVc<Box<dyn Issue>>>);
 #[derive(Debug)]
 pub struct CapturedIssues {
     issues: AutoSet<ResolvedVc<Box<dyn Issue>>>,
-    tracer: Arc<DelegatingImportTracer>,
+    tracer: ResolvedVc<DelegatingImportTracer>,
 }
 
 #[turbo_tasks::value_impl]
@@ -250,14 +247,12 @@ impl CapturedIssues {
 
     // Returns all the issues as formatted `PlainIssues`.
     pub async fn get_plain_issues(&self) -> Result<Vec<ReadRef<PlainIssue>>> {
-        let mut list =
-            self.issues
-                .iter()
-                .map(|issue| async move {
-                    PlainIssue::from_issue(**issue, Some(self.tracer.clone())).await
-                })
-                .try_join()
-                .await?;
+        let mut list = self
+            .issues
+            .iter()
+            .map(|issue| async move { PlainIssue::from_issue(**issue, Some(*self.tracer)).await })
+            .try_join()
+            .await?;
         list.sort();
         Ok(list)
     }
@@ -716,7 +711,7 @@ impl PlainIssue {
     #[turbo_tasks::function]
     pub async fn from_issue(
         issue: ResolvedVc<Box<dyn Issue>>,
-        import_tracer: Option<Arc<DelegatingImportTracer>>,
+        import_tracer: Option<ResolvedVc<DelegatingImportTracer>>,
     ) -> Result<Vc<Self>> {
         let description: Option<StyledString> = match *issue.description().await? {
             Some(description) => Some(description.owned().await?),
@@ -747,8 +742,13 @@ impl PlainIssue {
             },
             import_traces: match import_tracer {
                 Some(tracer) => {
-                    into_plain_trace(tracer.get_traces(issue.file_path().owned().await?).await?)
-                        .await?
+                    into_plain_trace(
+                        tracer
+                            .await?
+                            .get_traces(issue.file_path().owned().await?)
+                            .await?,
+                    )
+                    .await?
                 }
                 None => vec![],
             },
@@ -796,16 +796,14 @@ pub trait IssueReporter {
     ///
     /// # Arguments:
     ///
-    /// * `issues` - A [ReadRef] of [CapturedIssues]. Typically obtained with
-    ///   `source.peek_issues()`.
     /// * `source` - The root [Vc] from which issues are traced. Can be used by implementers to
-    ///   determine which issues are new.
+    ///   determine which issues are new.  This must be derived from the OperationVc so issues can
+    ///   be collected.
     /// * `min_failing_severity` - The minimum Vc<[IssueSeverity]>
     ///  The minimum issue severity level considered to fatally end the program.
     #[turbo_tasks::function]
     fn report_issues(
         self: Vc<Self>,
-        issues: TransientInstance<CapturedIssues>,
         source: TransientValue<RawVc>,
         min_failing_severity: IssueSeverity,
     ) -> Vc<bool>;
@@ -815,18 +813,17 @@ pub trait CollectibleIssuesExt
 where
     Self: Sized,
 {
-    /// Returns all issues from `source` in a list with their associated
-    /// processing path.
+    /// Returns all issues from `source`
+    ///
+    /// Must be called in a turbo-task as this constructs a `cell`
     fn peek_issues(self) -> CapturedIssues;
 
-    /// Returns all issues from `source` in a list with their associated
-    /// processing path.
+    /// Drops all issues from `source`
     ///
     /// This unemits the issues. They will not propagate up.
-    fn take_issues(self) -> CapturedIssues;
+    fn drop_issues(self);
 }
 
-#[async_trait]
 impl<T> CollectibleIssuesExt for T
 where
     T: CollectiblesSource + Copy + Send,
@@ -835,23 +832,21 @@ where
         CapturedIssues {
             issues: self.peek_collectibles(),
 
-            tracer: Arc::new(DelegatingImportTracer {
+            tracer: DelegatingImportTracer {
                 delegates: self.peek_collectibles(),
-            }),
+            }
+            .resolved_cell(),
         }
     }
 
-    fn take_issues(self) -> CapturedIssues {
-        CapturedIssues {
-            issues: self.take_collectibles(),
-
-            tracer: Arc::new(DelegatingImportTracer {
-                delegates: self.take_collectibles(),
-            }),
-        }
+    fn drop_issues(self) {
+        let _ = self.take_collectibles::<Box<dyn Issue>>();
     }
 }
 
+/// A helper function to print out issues to the console.
+///
+/// Must be called in a turbo-task as this constructs a `cell`
 pub async fn handle_issues<T: Send>(
     source_op: OperationVc<T>,
     issue_reporter: Vc<Box<dyn IssueReporter>>,
@@ -861,10 +856,8 @@ pub async fn handle_issues<T: Send>(
 ) -> Result<()> {
     let source_vc = source_op.connect();
     let _ = source_op.resolve_strongly_consistent().await?;
-    let issues = source_op.peek_issues();
 
     let has_fatal = issue_reporter.report_issues(
-        TransientInstance::new(issues),
         TransientValue::new(Vc::into_raw(source_vc)),
         min_failing_severity,
     );