Skip to content

Commit

Permalink
refactor(turbo-tasks): Derive NonLocalValue by default in value/value…
Browse files Browse the repository at this point in the history
…_trait macros (#73766)

**Recommendation:** Review [the individual commits](https://github.com/vercel/next.js/pull/73766/commits) in order!

**What is NonLocalValue?** https://turbopack-rust-docs.vercel.sh/rustdoc/turbo_tasks/trait.NonLocalValue.html

1. Switches the default behavior of `#[turbo_tasks::value]` and `#[turbo_tasks::value_trait]` to derive `NonLocalValue`, with an opt-out (previously, this was just opt-in).
2. *(Automated, codemod)* Remove pre-existing opt-ins to `NonLocalValue` (as this is now the default behavior)
3. Manually add `NonLocalValue` to a couple more common types.
4. *(Automated, codemod)* Add `local` annotations to structs and traits with errors.

Normally I'd split this sort of thing up across a few PRs, but steps 1, 2, and 4 must be atomic (must land together).

---

Removing pre-existing opt-ins was done with a couple regexes:

```
fastmod --accept-all '#\[turbo_tasks::value(?<trait>_trait)?\(((?<prefix>.*), )?non_local(?<suffix>.*)\)\]' '#[turbo_tasks::value${trait}(${prefix}${suffix})]'
fastmod --accept-all '#\[turbo_tasks::value(?<trait>_trait)?\(\)\]' '#[turbo_tasks::value${trait}]'
```

Adding `local` annotations to structs/enums and traits was done with this kludge (partially sourced from chatgpt):

```
cargo check --message-format=json 2>/dev/null | jq -r '. | select(.message.children != null) | .message.children[] | select(.spans != null) | .spans[] | select(.expansion.macro_decl_name == "#[derive(turbo_tasks::NonLocalValue)]") | .file_name + " " + (.line_start|tostring) + " " + (.line_end|tostring)' | sort -u | xargs -I{} bash -c 'args=($1); file="${args[0]}"; line="${args[1]}"; echo "Processing: $file $line" >&2; sed -i "${line}s/#\[turbo_tasks::value(\([^)]*\))\]/#[turbo_tasks::value(\1, local)]/; ${line}s/#\[turbo_tasks::value\]/#[turbo_tasks::value(local)]/" "$file"' -- {} && \
cargo check --message-format=json 2>/dev/null | jq -r '. | select(.message.children != null) | .message.children[] | select(.spans != null) | .spans[] | select(.expansion.macro_decl_name == "#[turbo_tasks::value_trait]") | .file_name + " " + (.line_start|tostring) + " " + (.line_end|tostring)' | sort -u | xargs -I{} bash -c 'args=($1); file="${args[0]}"; line="${args[1]}"; echo "Processing: $file $line" >&2; sed -i "${line}s/#\[turbo_tasks::value_trait(\([^)]*\))\]/#[turbo_tasks::value_trait(\1, local)]/; ${line}s/#\[turbo_tasks::value_trait\]/#[turbo_tasks::value_trait(local)]/" "$file"' -- {}
```

Which I just ran a bunch of times until `cargo check` no longer generated any errors.
  • Loading branch information
bgw authored Dec 12, 2024
1 parent a1ea8c7 commit 0ad304c
Show file tree
Hide file tree
Showing 87 changed files with 159 additions and 159 deletions.
2 changes: 1 addition & 1 deletion crates/napi/src/next_api/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ struct NapiEntrypoints {
pub pages_error_endpoint: External<ExternalEndpoint>,
}

#[turbo_tasks::value(serialization = "none")]
#[turbo_tasks::value(serialization = "none", local)]
struct EntrypointsWithIssues {
entrypoints: ReadRef<Entrypoints>,
issues: Arc<Vec<ReadRef<PlainIssue>>>,
Expand Down
2 changes: 1 addition & 1 deletion crates/next-api/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ enum AppEndpointType {
},
}

#[turbo_tasks::value]
#[turbo_tasks::value(local)]
struct AppEndpoint {
ty: AppEndpointType,
app_project: ResolvedVc<AppProject>,
Expand Down
2 changes: 1 addition & 1 deletion crates/next-api/src/entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
route::{Endpoint, Route},
};

#[turbo_tasks::value(shared)]
#[turbo_tasks::value(shared, local)]
pub struct Entrypoints {
pub routes: FxIndexMap<RcStr, Route>,
pub middleware: Option<Middleware>,
Expand Down
2 changes: 1 addition & 1 deletion crates/next-api/src/module_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<N: TraceRawVcs, E: TraceRawVcs> Deref for TracedDiGraph<N, E> {
}
}

#[turbo_tasks::value(cell = "new", eq = "manual", into = "new")]
#[turbo_tasks::value(cell = "new", eq = "manual", into = "new", local)]
#[derive(Clone, Default)]
pub struct SingleModuleGraph {
graph: TracedDiGraph<SingleModuleGraphNode, ()>,
Expand Down
2 changes: 1 addition & 1 deletion crates/next-api/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ pub struct Instrumentation {
pub edge: Vc<Box<dyn Endpoint>>,
}

#[turbo_tasks::value]
#[turbo_tasks::value(local)]
pub struct ProjectContainer {
name: RcStr,
options_state: State<Option<ProjectOptions>>,
Expand Down
6 changes: 3 additions & 3 deletions crates/next-api/src/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl AppPageRoute {
}
}

#[turbo_tasks::value(shared)]
#[turbo_tasks::value(shared, local)]
#[derive(Clone, Debug)]
pub enum Route {
Page {
Expand Down Expand Up @@ -58,7 +58,7 @@ impl Route {
}
}

#[turbo_tasks::value_trait]
#[turbo_tasks::value_trait(local)]
pub trait Endpoint {
fn write_to_disk(self: Vc<Self>) -> Vc<WrittenEndpoint>;
fn server_changed(self: Vc<Self>) -> Vc<Completion>;
Expand All @@ -83,5 +83,5 @@ pub enum WrittenEndpoint {

/// The routes as map from pathname to route. (pathname includes the leading
/// slash)
#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
pub struct Routes(FxIndexMap<RcStr, Route>);
6 changes: 3 additions & 3 deletions crates/next-api/src/versioned_content_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use turbopack_core::{
/// An unresolved output assets operation. We need to pass an operation here as
/// it's stored for later usage and we want to reconnect this operation when
/// it's received from the map again.
#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
pub struct OutputAssetsOperation(Vc<OutputAssets>);

#[derive(Clone, TraceRawVcs, PartialEq, Eq, ValueDebugFormat, Serialize, Deserialize, Debug)]
Expand All @@ -30,14 +30,14 @@ struct MapEntry {
path_to_asset: HashMap<ResolvedVc<FileSystemPath>, Vc<Box<dyn OutputAsset>>>,
}

#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
struct OptionMapEntry(Option<MapEntry>);

type PathToOutputOperation = HashMap<ResolvedVc<FileSystemPath>, FxIndexSet<Vc<OutputAssets>>>;
// A precomputed map for quick access to output asset by filepath
type OutputOperationToComputeEntry = HashMap<Vc<OutputAssets>, Vc<OptionMapEntry>>;

#[turbo_tasks::value]
#[turbo_tasks::value(local)]
pub struct VersionedContentMap {
// TODO: turn into a bi-directional multimap, OutputAssets -> FxIndexSet<FileSystemPath>
map_path_to_op: State<PathToOutputOperation>,
Expand Down
14 changes: 7 additions & 7 deletions crates/next-core/src/app_structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::{
};

/// A final route in the app directory.
#[turbo_tasks::value]
#[turbo_tasks::value(local)]
#[derive(Default, Debug, Clone)]
pub struct AppDirModules {
#[serde(skip_serializing_if = "Option::is_none")]
Expand Down Expand Up @@ -180,7 +180,7 @@ impl Metadata {
}

/// Metadata files that can be placed in the root of the app directory.
#[turbo_tasks::value]
#[turbo_tasks::value(local)]
#[derive(Default, Clone, Debug)]
pub struct GlobalMetadata {
#[serde(skip_serializing_if = "Option::is_none")]
Expand All @@ -202,15 +202,15 @@ impl GlobalMetadata {
}
}

#[turbo_tasks::value]
#[turbo_tasks::value(local)]
#[derive(Debug)]
pub struct DirectoryTree {
/// key is e.g. "dashboard", "(dashboard)", "@slot"
pub subdirectories: BTreeMap<RcStr, ResolvedVc<DirectoryTree>>,
pub modules: AppDirModules,
}

#[turbo_tasks::value]
#[turbo_tasks::value(local)]
#[derive(Clone, Debug)]
struct PlainDirectoryTree {
/// key is e.g. "dashboard", "(dashboard)", "@slot"
Expand Down Expand Up @@ -400,7 +400,7 @@ async fn get_directory_tree_internal(
.cell())
}

#[turbo_tasks::value]
#[turbo_tasks::value(local)]
#[derive(Debug, Clone)]
pub struct AppPageLoaderTree {
pub page: AppPage,
Expand Down Expand Up @@ -523,7 +523,7 @@ impl Entrypoint {
}
}

#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
pub struct Entrypoints(FxIndexMap<AppPath, Entrypoint>);

fn is_parallel_route(name: &str) -> bool {
Expand Down Expand Up @@ -1411,7 +1411,7 @@ pub async fn get_global_metadata(
Ok(metadata.cell())
}

#[turbo_tasks::value(shared)]
#[turbo_tasks::value(shared, local)]
struct DirectoryTreeIssue {
pub severity: ResolvedVc<IssueSeverity>,
// no-resolved-vc(kdy1): I'll resolve this later because it's a complex case.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl Default for ClientReferenceGraphResult {
}
}

#[turbo_tasks::value(shared)]
#[turbo_tasks::value(shared, local)]
pub struct VisitedClientReferenceGraphNodes(HashSet<VisitClientReferenceNode>);

#[turbo_tasks::value_impl]
Expand Down
6 changes: 3 additions & 3 deletions crates/next-core/src/next_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ pub enum LoaderItem {
LoaderOptions(WebpackLoaderItem),
}

#[turbo_tasks::value(non_local)]
#[turbo_tasks::value]
#[derive(Clone, Debug)]
#[serde(rename_all = "camelCase")]
pub enum ModuleIdStrategy {
Expand All @@ -466,7 +466,7 @@ pub enum MdxRsOptions {
Option(MdxTransformOptions),
}

#[turbo_tasks::value(shared, non_local)]
#[turbo_tasks::value(shared)]
#[derive(Clone, Debug)]
#[serde(rename_all = "camelCase")]
pub enum ReactCompilerMode {
Expand All @@ -476,7 +476,7 @@ pub enum ReactCompilerMode {
}

/// Subset of react compiler options
#[turbo_tasks::value(shared, non_local)]
#[turbo_tasks::value(shared)]
#[derive(Clone, Debug)]
#[serde(rename_all = "camelCase")]
pub struct ReactCompilerOptions {
Expand Down
2 changes: 1 addition & 1 deletion crates/next-core/src/next_server/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ pub struct PackagesGlobs {
}

// TODO move that to turbo
#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
pub struct OptionPackagesGlobs(Option<PackagesGlobs>);

#[turbo_tasks::function]
Expand Down
2 changes: 1 addition & 1 deletion turbopack/crates/turbo-tasks-fs/src/attach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
/// "subdirectory" in the given root [FileSystem].
///
/// Caveat: The `child_path` itself is not visible as a directory entry.
#[turbo_tasks::value(non_local)]
#[turbo_tasks::value]
pub struct AttachedFileSystem {
root_fs: ResolvedVc<Box<dyn FileSystem>>,
// we turn this into a string because creating a FileSystemPath requires the filesystem which
Expand Down
2 changes: 1 addition & 1 deletion turbopack/crates/turbo-tasks-fs/src/embed/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
LinkContent,
};

#[turbo_tasks::value(serialization = "none", cell = "new", eq = "manual", non_local)]
#[turbo_tasks::value(serialization = "none", cell = "new", eq = "manual")]
pub struct EmbeddedFileSystem {
name: RcStr,
#[turbo_tasks(trace_ignore)]
Expand Down
2 changes: 1 addition & 1 deletion turbopack/crates/turbo-tasks-fs/src/glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ enum GlobPart {
// Note: a/**/b does match a/b, so we need some special logic about path
// separators

#[turbo_tasks::value(non_local)]
#[turbo_tasks::value]
#[derive(Debug, Clone)]
pub struct Glob {
expression: Vec<GlobPart>,
Expand Down
12 changes: 6 additions & 6 deletions turbopack/crates/turbo-tasks-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub fn validate_path_length(path: &Path) -> Result<Cow<'_, Path>> {
})
}

#[turbo_tasks::value_trait(non_local)]
#[turbo_tasks::value_trait]
pub trait FileSystem: ValueToString {
/// Returns the path to the root of the file system.
fn root(self: Vc<Self>) -> Vc<FileSystemPath> {
Expand Down Expand Up @@ -348,7 +348,7 @@ impl DiskFileSystemInner {
}
}

#[turbo_tasks::value(cell = "new", eq = "manual", non_local)]
#[turbo_tasks::value(cell = "new", eq = "manual")]
pub struct DiskFileSystem {
inner: Arc<DiskFileSystemInner>,
}
Expand Down Expand Up @@ -944,7 +944,7 @@ impl ValueToString for DiskFileSystem {
}
}

#[turbo_tasks::value(non_local)]
#[turbo_tasks::value]
#[derive(Debug, Clone)]
pub struct FileSystemPath {
pub fs: ResolvedVc<Box<dyn FileSystem>>,
Expand Down Expand Up @@ -1702,7 +1702,7 @@ impl FileContent {
}

bitflags! {
#[derive(Default, Serialize, Deserialize, TraceRawVcs)]
#[derive(Default, Serialize, Deserialize, TraceRawVcs, NonLocalValue)]
pub struct LinkType: u8 {
const DIRECTORY = 0b00000001;
const ABSOLUTE = 0b00000010;
Expand Down Expand Up @@ -2100,7 +2100,7 @@ impl FileContent {
}

/// A file's content interpreted as a JSON value.
#[turbo_tasks::value(shared, serialization = "none", non_local)]
#[turbo_tasks::value(shared, serialization = "none")]
pub enum FileJsonContent {
Content(Value),
Unparseable(Box<UnparseableJson>),
Expand Down Expand Up @@ -2279,7 +2279,7 @@ impl DirectoryContent {
}
}

#[turbo_tasks::value(shared, non_local)]
#[turbo_tasks::value(shared)]
pub struct NullFileSystem;

#[turbo_tasks::value_impl]
Expand Down
2 changes: 1 addition & 1 deletion turbopack/crates/turbo-tasks-fs/src/virtual_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use turbo_tasks::{Completion, ValueDefault, ValueToString, Vc};

use super::{DirectoryContent, FileContent, FileMeta, FileSystem, FileSystemPath, LinkContent};

#[turbo_tasks::value(non_local)]
#[turbo_tasks::value]
pub struct VirtualFileSystem {
pub name: RcStr,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@ pub struct ValueTraitArguments {
/// Whether the macro should generate a `ValueDebug`-like `dbg()`
/// implementation on the trait's `Vc`.
pub debug: bool,
/// Should the trait have a `turbo_tasks::NonLocalValue` constraint?
///
/// `Some(...)` if enabled, containing the span that enabled the constraint.
pub non_local: Option<Span>,
/// Should the trait have a `turbo_tasks::OperationValue` constraint?
/// By default, traits have a `turbo_tasks::NonLocalValue` supertype. Should we skip this?
pub local: bool,
/// Should the trait have a `turbo_tasks::OperationValue` supertype?
pub operation: Option<Span>,
}

impl Default for ValueTraitArguments {
fn default() -> Self {
Self {
debug: true,
non_local: None,
local: false,
operation: None,
}
}
Expand All @@ -43,8 +41,8 @@ impl Parse for ValueTraitArguments {
Some("no_debug") => {
result.debug = false;
}
Some("non_local") => {
result.non_local = Some(meta.span());
Some("local") => {
result.local = true;
}
Some("operation") => {
result.operation = Some(meta.span());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use turbo_tasks::{ResolvedVc, Vc};

#[turbo_tasks::value(transparent, non_local)]
#[turbo_tasks::value(transparent)]
struct IntegersVec(Vec<ResolvedVc<u32>>);

#[turbo_tasks::function(invalid_argument)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use turbo_tasks::{ResolvedVc, Vc};
#[turbo_tasks::value]
struct ExampleStruct;

#[turbo_tasks::value(transparent, non_local)]
#[turbo_tasks::value(transparent)]
struct IntegersVec(Vec<ResolvedVc<u32>>);

#[turbo_tasks::value_impl]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use turbo_tasks::Vc;
#[turbo_tasks::value]
struct ExampleStruct;

#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
struct IntegersVec(Vec<Vc<u32>>);

Check warning on line 11 in turbopack/crates/turbo-tasks-macros-tests/tests/function/fail_non_local_inherent_impl.rs

View workflow job for this annotation

GitHub Actions / ast-grep lint

no-vc-struct

Don't use a Vc directly in a struct

#[turbo_tasks::value_impl]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use turbo_tasks::Vc;

#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
struct IntegersVec(Vec<Vc<u32>>);

Check warning on line 8 in turbopack/crates/turbo-tasks-macros-tests/tests/function/fail_non_local_static.rs

View workflow job for this annotation

GitHub Actions / ast-grep lint

no-vc-struct

Don't use a Vc directly in a struct

#[turbo_tasks::function(non_local_return)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use turbo_tasks::Vc;
#[turbo_tasks::value]
struct ExampleStruct;

#[turbo_tasks::value(transparent)]
#[turbo_tasks::value(transparent, local)]
struct IntegersVec(Vec<Vc<u32>>);

Check warning on line 11 in turbopack/crates/turbo-tasks-macros-tests/tests/function/fail_non_local_trait_impl.rs

View workflow job for this annotation

GitHub Actions / ast-grep lint

no-vc-struct

Don't use a Vc directly in a struct

#[turbo_tasks::value_trait]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use turbo_tasks::{ResolvedVc, Vc};
#[turbo_tasks::value]
struct ExampleStruct;

#[turbo_tasks::value(transparent, non_local)]
#[turbo_tasks::value(transparent)]
struct IntegersVec(Vec<ResolvedVc<u32>>);

#[turbo_tasks::value_impl]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use turbo_tasks::{ResolvedVc, Vc};

#[turbo_tasks::value(transparent, non_local)]
#[turbo_tasks::value(transparent)]
struct IntegersVec(Vec<ResolvedVc<u32>>);

#[turbo_tasks::function(non_local_return)]
Expand Down
Loading

0 comments on commit 0ad304c

Please sign in to comment.