Repository: vercel/next.js
Pull request: fix(turbopack): Fix static immutability analysis

### What?

Do not mark functions with `(&self)` receiver as imuutable.

### Why?

`&self` is `self: Vc<Self>` with `self.await?`

Closes PACK-4860

Changed files:
filename: turbopack/crates/turbo-tasks-macros/src/function_macro.rs
patch: @@ -67,7 +67,7 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
         filter_trait_call_args: None, // not a trait method
         local,
         invalidator,
-        immutable: sig.asyncness.is_none() && !invalidator,
+        immutable: is_immutable(&sig) && !invalidator,
     };
     let native_function_ident = get_native_function_ident(ident);
     let native_function_ty = native_fn.ty();
@@ -102,3 +102,15 @@ pub fn function(args: TokenStream, input: TokenStream) -> TokenStream {
     }
     .into()
 }
+
+/// Computes whether the task is statically immutable based on the signature.
+/// - if the task is `async` we assume it is reading some other task.
+/// - if a task accepts `&self` then we know it read `Vc<Self>` in the generated calling code. See
+///   also: turbopack/crates/turbo-tasks/src/task/function.rs for the binding code.
+pub(crate) fn is_immutable(sig: &syn::Signature) -> bool {
+    sig.asyncness.is_none()
+        && match sig.receiver() {
+            Some(recv) => recv.reference.is_none(),
+            _ => true,
+        }
+}

filename: turbopack/crates/turbo-tasks-macros/src/value_impl_macro.rs
patch: @@ -14,9 +14,12 @@ use turbo_tasks_macros_shared::{
     get_trait_impl_function_id_ident, get_trait_impl_function_ident, get_type_ident, is_self_used,
 };
 
-use crate::func::{
-    DefinitionContext, FunctionArguments, NativeFn, TurboFn, filter_inline_attributes,
-    split_function_attributes,
+use crate::{
+    func::{
+        DefinitionContext, FunctionArguments, NativeFn, TurboFn, filter_inline_attributes,
+        split_function_attributes,
+    },
+    function_macro::is_immutable,
 };
 
 struct ValueImplArguments {
@@ -119,7 +122,7 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
                     filter_trait_call_args: None, // not a trait method
                     local,
                     invalidator,
-                    immutable: sig.asyncness.is_none() && !invalidator,
+                    immutable: is_immutable(sig) && !invalidator,
                 };
 
                 let native_function_ident = get_inherent_impl_function_ident(ty_ident, ident);
@@ -249,7 +252,7 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
                     filter_trait_call_args: turbo_fn.filter_trait_call_args(),
                     local,
                     invalidator,
-                    immutable: sig.asyncness.is_none() && !invalidator,
+                    immutable: is_immutable(sig) && !invalidator,
                 };
 
                 let native_function_ident =

filename: turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs
patch: @@ -10,9 +10,12 @@ use turbo_tasks_macros_shared::{
     get_trait_type_vtable_registry, is_self_used,
 };
 
-use crate::func::{
-    DefinitionContext, FunctionArguments, NativeFn, TurboFn, filter_inline_attributes,
-    split_function_attributes,
+use crate::{
+    func::{
+        DefinitionContext, FunctionArguments, NativeFn, TurboFn, filter_inline_attributes,
+        split_function_attributes,
+    },
+    function_macro::is_immutable,
 };
 
 pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
@@ -194,7 +197,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
                 // - This only makes sense when a default implementation is present.
                 local: false,
                 invalidator: func_args.invalidator.is_some(),
-                immutable: sig.asyncness.is_none() && func_args.invalidator.is_none(),
+                immutable: is_immutable(sig) && func_args.invalidator.is_none(),
             };
 
             let native_function_ident = get_trait_default_impl_function_ident(trait_ident, ident);

filename: turbopack/crates/turbo-tasks-testing/tests/recompute.rs
patch: @@ -3,7 +3,7 @@
 #![allow(clippy::needless_return)] // tokio macro-generated code doesn't respect this
 
 use anyhow::Result;
-use turbo_tasks::{State, Vc};
+use turbo_tasks::{ResolvedVc, State, Vc};
 use turbo_tasks_testing::{Registration, register, run};
 
 static REGISTRATION: Registration = register!();
@@ -58,11 +58,53 @@ async fn recompute() {
     .unwrap()
 }
 
+#[tokio::test]
+async fn immutable_analysis() {
+    run(&REGISTRATION, || async {
+        let input = ChangingInput {
+            state: State::new(1),
+        }
+        .resolved_cell();
+
+        // Verify
+
+        let vc_holder = VcHolder { vc: input }.resolved_cell();
+        let read = vc_holder.compute().strongly_consistent().await?;
+        assert_eq!(read.state_value, 1);
+        assert_eq!(read.state_value2, 1);
+        let random_value = read.random_value;
+
+        println!("changing input1");
+        input.await?.state.set(30);
+        let read = vc_holder.compute().strongly_consistent().await?;
+        assert_eq!(read.state_value, 30);
+        assert_eq!(read.state_value2, 42);
+        assert_ne!(read.random_value, random_value);
+
+        anyhow::Ok(())
+    })
+    .await
+    .unwrap()
+}
+
 #[turbo_tasks::value]
 struct ChangingInput {
     state: State<u32>,
 }
 
+#[turbo_tasks::value]
+struct VcHolder {
+    vc: ResolvedVc<ChangingInput>,
+}
+
+#[turbo_tasks::value_impl]
+impl VcHolder {
+    #[turbo_tasks::function]
+    fn compute(&self) -> Vc<Output> {
+        compute(*self.vc, *self.vc)
+    }
+}
+
 #[turbo_tasks::value]
 struct Output {
     state_value: u32,