-
Notifications
You must be signed in to change notification settings - Fork 26.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(turbo-tasks): Allow
#[turbo_tasks::function]
s to accept Resolv…
…edVc types as arguments (#70269) ## Why? [We want to codemod structs to use `ResolvedVc<...>` for all of their field types instead of `Vc<...>`.](https://www.notion.so/vercel/Resolved-Vcs-Vc-Lifetimes-Local-Vcs-and-Vc-Refcounts-49d666d3f9594017b5b312b87ddc5bff?pvs=4) There are many constructor-like functions (see #70133) where we must accept an argument of type `Vc<...>`, and then explicitly call `.to_resolved().await?`. However, internally `#[turbo_tasks::function]` arguments are guaranteed to be resolved by the time the function runs. So we can do a cheap conversion here. ## What Instead of needing to do: ```diff #[turbo_tasks::value_impl] impl CustomProcessEnv { #[turbo_tasks::function] - pub fn new(prior: Vc<Box<dyn ProcessEnv>>, custom: Vc<EnvMap>) -> Vc<Self> { - CustomProcessEnv { prior, custom }.cell() + pub async fn new(prior: Vc<Box<dyn ProcessEnv>>, custom: Vc<EnvMap>) -> Result<Vc<Self>> { + let prior = prior.to_resolved().await?; + let custom = custom.to_resolved().await?; + Ok(CustomProcessEnv { prior, custom }.cell()) } } ``` It should now just be possible to accept `ResolvedVc` instead. The exposed function signature will be unchanged, still accepting `Vc` arguments, and a conversion will happen internally. ```diff #[turbo_tasks::value_impl] impl CustomProcessEnv { #[turbo_tasks::function] - pub fn new(prior: Vc<Box<dyn ProcessEnv>>, custom: Vc<EnvMap>) -> Vc<Self> { + pub fn new(prior: ResolvedVc<Box<dyn ProcessEnv>>, custom: ResolvedVc<EnvMap>) ->Vc<Self> { CustomProcessEnv { prior, custom }.cell() } } ``` This should also work for arguments where `Vc` is inside of a `Vec` or `Option` (other collection types are not currently supported). This PR does not support `self` arguments. That is handled by #70367. ## How - The macro inspects the argument type and rewrites it to replace `ResolvedVc` with `Vc` to get the exposed function's signature. - The `FromTaskInput` trait does the actual conversion. ### Why do this type expansion and conversion in the macro, and not as part of [the `TaskFn` trait](https://github.com/vercel/next.js/blob/8f9c6a86177513026ab4bc4fdc3575ca1efe025c/turbopack/crates/turbo-tasks/src/task/function.rs)? Without [specialization](https://github.com/rust-lang/rfcs/blob/master/text/1210-impl-specialization.md) it's not possible to implement the `FromTaskInput` trait for all `TaskInput` types, as we'd end up with overlapping impls for `Option<T>` and `Vec<T>`. There are specialization hacks ([inherent method specialization](dtolnay/case-studies#14), [autoref-specialization, and autoderef-specialization](http://lukaskalbertodt.github.io/2019/12/05/generalized-autoref-based-specialization.html)) but those hacks are mostly for macros, not for generic code: > One thing might be worth clarifying up front: the adopted version described here does not solve *the* main limitation of autoref-based specialization, namely specializing in a generic context. For example, given `fn foo<T: Clone>()`, you cannot specialize for `T: Copy` in that function with autoref-based specialization. For these kinds of parametricity-destroying cases, “real” specialization is still required. As such, the whole autoref-based specialization technique is still mainly relevant for usage with macros. So we need the macro to determine if a type implements `FromTaskInput` or `TaskInput`. We can't do this inside of generic function. Aside from that, even though it's not as technically correct, expanding the types inside the macro results in *much* more readable types in rustdoc, which is why we do this in `expand_vc_return_type` as well, even though we could use a trait's associated type instead: vercel/turborepo#8096 ## Test Plan ``` cargo nextest r -p turbo-tasks-memory test_resolved_vc cargo nextest r -p turbo-tasks-macros-tests function ``` Modify some code to use this, and use `rust-analyzer`'s macro expansion feature (after telling RA to rebuild proc macros).
- Loading branch information
Showing
9 changed files
with
389 additions
and
74 deletions.
There are no files selected for viewing
40 changes: 40 additions & 0 deletions
40
turbopack/crates/turbo-tasks-macros-tests/tests/function/pass_resolved_vc_input.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
#![feature(arbitrary_self_types)] | ||
#![allow(dead_code)] | ||
|
||
use anyhow::Result; | ||
use turbo_tasks::{ResolvedVc, Vc}; | ||
|
||
#[derive(Clone)] | ||
#[turbo_tasks::value(resolved)] | ||
struct ExampleStruct { | ||
items: Vec<ResolvedVc<u32>>, | ||
} | ||
|
||
#[turbo_tasks::value_impl] | ||
impl ExampleStruct { | ||
#[turbo_tasks::function] | ||
fn constructor_one(item: ResolvedVc<u32>) -> Vc<Self> { | ||
ExampleStruct { items: vec![item] }.cell() | ||
} | ||
|
||
#[turbo_tasks::function] | ||
fn constructor_vec(items: Vec<turbo_tasks::ResolvedVc<u32>>) -> Vc<Self> { | ||
ExampleStruct { items }.cell() | ||
} | ||
} | ||
|
||
#[turbo_tasks::value(resolved, transparent)] | ||
struct MaybeExampleStruct(Option<ExampleStruct>); | ||
|
||
#[turbo_tasks::function] | ||
async fn caller_uses_unresolved_vc(items: Option<Vec<Vc<u32>>>) -> Result<Vc<MaybeExampleStruct>> { | ||
if let Some(items) = items { | ||
// call `constructor_vec` with `Vc` (not `ResolvedVc`) | ||
let inner = ExampleStruct::constructor_vec(items).await?; | ||
Ok(Vc::cell(Some((*inner).clone()))) | ||
} else { | ||
Ok(Vc::cell(None)) | ||
} | ||
} | ||
|
||
fn main() {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.