[turbopack] Drop turbo_tasks::function from EcmascriptAnalyzable::module_content (#83277)

This function simply plugs two other VCs together, and it never gets a cache hit because it is "dominated" by other tasks.

The only reason it is a turbotask is because it is defined as a trait item, however, we recently (#79217) allowed for value_trait items to not be turbo tasks.  This PR builds on that to additionally allow for non-turbo_task trait items to accept `Vc<Self>`.  This only really works if there is also a default implementation which works for this case.

A tiny speed up progression in production

![image.png](https://app.graphite.dev/user-attachments/assets/c6d82929-5fb2-4921-b566-d648751a00ee.png)

Author: lukesandberg

turbopack/crates/turbo-tasks-macros/src/func.rs

@@ -108,46 +108,8 @@ impl TurboFn<'_> {
                         _ => &definition_context,
                     };
 
-                    match self_type.as_ref() {
-                        // we allow `&Self` but not `&mut Self`
-                        syn::Type::Reference(type_reference) => {
-                            if let Some(m) = type_reference.mutability {
-                                m.span()
-                                    .unwrap()
-                                    .error(format!(
-                                        "{} cannot take self by mutable reference, use &self or \
-                                         self: Vc<Self> instead",
-                                        definition_context.function_type(),
-                                    ))
-                                    .emit();
-                                return None;
-                            }
-
-                            match type_reference.elem.as_ref() {
-                                syn::Type::Path(TypePath { qself: None, path })
-                                    if path.is_ident("Self") => {}
-                                _ => {
-                                    self_type
-                                        .span()
-                                        .unwrap()
-                                        .error(
-                                            "Unexpected `self` type, use `&self` or `self: \
-                                             Vc<Self>",
-                                        )
-                                        .emit();
-                                    return None;
-                                }
-                            }
-                        }
-                        syn::Type::Path(_) => {}
-                        _ => {
-                            self_type
-                                .span()
-                                .unwrap()
-                                .error("Unexpected `self` type, use `&self` or `self: Vc<Self>")
-                                .emit();
-                            return None;
-                        }
+                    if get_receiver_style(self_type, definition_context) == ReceiverStyle::Error {
+                        return None;
                     }
                     // We don't validate that the user provided a valid `turbo_tasks::Vc<Self>`
                     // here. We'll rely on the compiler to emit an error if the user provided an
@@ -695,6 +657,62 @@ impl TurboFn<'_> {
     }
 }
 
+#[derive(PartialEq, Eq)]
+pub enum ReceiverStyle {
+    // A reference like &self or self: &Self
+    Reference,
+    // A Vc<> type, this is optimistic
+    Vc,
+    Error,
+}
+
+pub(crate) fn get_receiver_style(
+    self_type: &Type,
+    definition_context: &DefinitionContext,
+) -> ReceiverStyle {
+    match self_type {
+        // we allow `&Self` but not `&mut Self`
+        syn::Type::Reference(type_reference) => {
+            if let Some(m) = type_reference.mutability {
+                m.span()
+                    .unwrap()
+                    .error(format!(
+                        "{} cannot take self by mutable reference, use &self or self: Vc<Self> \
+                         instead",
+                        definition_context.function_type(),
+                    ))
+                    .emit();
+                return ReceiverStyle::Error;
+            }
+
+            match type_reference.elem.as_ref() {
+                syn::Type::Path(TypePath { qself: None, path }) if path.is_ident("Self") => {}
+                _ => {
+                    self_type
+                        .span()
+                        .unwrap()
+                        .error("Unexpected `self` type, use `&self` or `self: Vc<Self>")
+                        .emit();
+                    return ReceiverStyle::Error;
+                }
+            }
+            return ReceiverStyle::Reference;
+        }
+        syn::Type::Path(_) => {}
+        _ => {
+            self_type
+                .span()
+                .unwrap()
+                .error("Unexpected `self` type, use `&self` or `self: Vc<Self>")
+                .emit();
+            return ReceiverStyle::Error;
+        }
+    }
+    // All other cases are assumed to be a VC, this is not guaranteed but we are happy to just have
+    // compiler errors when this assumption is wrong.
+    ReceiverStyle::Vc
+}
+
 /// An indication of what kind of IO this function does. Currently only used for
 /// static analysis, and ignored within this macro.
 #[derive(Hash, PartialEq, Eq)]

turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs

@@ -2,7 +2,8 @@ use proc_macro::TokenStream;
 use proc_macro2::{Ident, TokenStream as TokenStream2};
 use quote::{quote, quote_spanned};
 use syn::{
-    FnArg, ItemTrait, Pat, TraitItem, TraitItemFn, parse_macro_input, parse_quote, spanned::Spanned,
+    FnArg, ItemTrait, Pat, Receiver, TraitItem, TraitItemFn, parse_macro_input, parse_quote,
+    spanned::Spanned,
 };
 use turbo_tasks_macros_shared::{
     ValueTraitArguments, get_trait_default_impl_function_ident, get_trait_type_ident, is_self_used,
@@ -11,7 +12,7 @@ use turbo_tasks_macros_shared::{
 use crate::{
     func::{
         DefinitionContext, FunctionArguments, NativeFn, TurboFn, filter_inline_attributes,
-        split_function_attributes,
+        get_receiver_style, split_function_attributes,
     },
     global_name::global_name,
 };
@@ -94,15 +95,29 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
         let (func_args, attrs) = split_function_attributes(attrs);
         let func_args = match func_args {
             Ok(None) => {
-                // There is no turbo_tasks::function annotation, preserve this item
+                // There is no turbo_tasks::function annotation, preserve this item as is in the
+                // trait
                 items.push(item.clone());
                 // But we still need to add a forwarding implementation to the
                 // impl for `turbo_tasks::Dynamic<Box<dyn T>>`
                 // This will have the same signature, but simply forward the call
                 let mut args = Vec::new();
+                let mut is_vc_receiver = false;
                 for arg in &sig.inputs {
                     let ident = match arg {
-                        FnArg::Receiver(_) => {
+                        FnArg::Receiver(Receiver { ty, .. }) => {
+                            match get_receiver_style(ty, &DefinitionContext::ValueTrait) {
+                                crate::func::ReceiverStyle::Reference => {
+                                    is_vc_receiver = false;
+                                }
+                                crate::func::ReceiverStyle::Vc => {
+                                    is_vc_receiver = true;
+                                }
+                                crate::func::ReceiverStyle::Error => {}
+                            }
+                            // We allow either `&self` or `self: Vc<Self>`
+                            // we cannot really validate Vc<Self> so instead we simply assume that
+                            // any type that isn't a reference is Vc<Self>
                             continue;
                         }
                         FnArg::Typed(pat) => match &*pat.pat {
@@ -120,8 +135,17 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
                     };
                     args.push(ident);
                 }
+                if is_vc_receiver {
+                    item.span()
+                        .unwrap()
+                        .error(
+                            "`self: Vc<Self>` is only supported on trait items with a \
+                             `turbo-tasks::function` annotation",
+                        )
+                        .emit();
+                }
                 // Add a dummy implementation that derefences the box and delegates to the
-                // actual implementation.
+                // actual implementation.  We need to conditionally add an await if it is async
                 dynamic_trait_fns.push(if sig.asyncness.is_some() {
                     quote! {
                         #sig {

turbopack/crates/turbo-tasks/src/vc/resolved.rs

@@ -243,7 +243,7 @@ where
     /// Returns `None` if the underlying value type does not implement `K`.
     ///
     /// **Note:** if the trait `T` is required to implement `K`, use [`ResolvedVc::upcast`] instead.
-    /// This provides stronger guarantees, removing the need for a [`Option`] return type.
+    /// That method provides stronger guarantees, removing the need for a [`Option`] return type.
     ///
     /// See also: [`Vc::try_resolve_sidecast`].
     pub fn try_sidecast<K>(this: Self) -> Option<ResolvedVc<K>>

turbopack/crates/turbopack-ecmascript/src/lib.rs

@@ -84,7 +84,7 @@ pub use transform::{
 use turbo_rcstr::{RcStr, rcstr};
 use turbo_tasks::{
     FxDashMap, FxIndexMap, IntoTraitRef, NonLocalValue, ReadRef, ResolvedVc, TaskInput,
-    TryFlatJoinIterExt, TryJoinIterExt, ValueToString, Vc, trace::TraceRawVcs,
+    TryFlatJoinIterExt, TryJoinIterExt, Upcast, ValueToString, Vc, trace::TraceRawVcs,
 };
 use turbo_tasks_fs::{FileJsonContent, FileSystemPath, glob::Glob, rope::Rope};
 use turbopack_core::{
@@ -367,15 +367,28 @@ pub trait EcmascriptAnalyzable: Module + Asset {
         chunking_context: Vc<Box<dyn ChunkingContext>>,
         async_module_info: Option<Vc<AsyncModuleInfo>>,
     ) -> Result<Vc<EcmascriptModuleContentOptions>>;
+}
 
-    #[turbo_tasks::function]
+pub trait EcmascriptAnalyzableExt {
     fn module_content(
         self: Vc<Self>,
         chunking_context: Vc<Box<dyn ChunkingContext>>,
         async_module_info: Option<Vc<AsyncModuleInfo>>,
-    ) -> Result<Vc<EcmascriptModuleContent>> {
-        let own_options = self.module_content_options(chunking_context, async_module_info);
-        Ok(EcmascriptModuleContent::new(own_options))
+    ) -> Vc<EcmascriptModuleContent>;
+}
+
+impl<T> EcmascriptAnalyzableExt for T
+where
+    T: EcmascriptAnalyzable + Upcast<Box<dyn EcmascriptAnalyzable>>,
+{
+    fn module_content(
+        self: Vc<Self>,
+        chunking_context: Vc<Box<dyn ChunkingContext>>,
+        async_module_info: Option<Vc<AsyncModuleInfo>>,
+    ) -> Vc<EcmascriptModuleContent> {
+        let analyzable = Vc::upcast_non_strict::<Box<dyn EcmascriptAnalyzable>>(self);
+        let own_options = analyzable.module_content_options(chunking_context, async_module_info);
+        EcmascriptModuleContent::new(own_options)
     }
 }
 

turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/chunk_item.rs

@@ -8,7 +8,7 @@ use turbopack_core::{
 
 use super::module::EcmascriptModuleFacadeModule;
 use crate::{
-    EcmascriptAnalyzable,
+    EcmascriptAnalyzableExt,
     chunk::{
         EcmascriptChunkItem, EcmascriptChunkItemContent, EcmascriptChunkPlaceable,
         EcmascriptChunkType,

turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/chunk_item.rs

@@ -8,7 +8,7 @@ use turbopack_core::{
 
 use super::module::EcmascriptModuleLocalsModule;
 use crate::{
-    EcmascriptAnalyzable,
+    EcmascriptAnalyzableExt,
     chunk::{EcmascriptChunkItem, EcmascriptChunkItemContent, EcmascriptChunkType},
 };
 

turbopack/crates/turbopack-ecmascript/src/tree_shake/chunk_item.rs

@@ -10,7 +10,7 @@ use turbopack_core::{
 
 use super::asset::EcmascriptModulePartAsset;
 use crate::{
-    EcmascriptAnalyzable,
+    EcmascriptAnalyzableExt,
     chunk::{
         EcmascriptChunkItem, EcmascriptChunkItemContent, EcmascriptChunkItemOptions,
         EcmascriptChunkPlaceable, EcmascriptChunkType,