chore(turbo-tasks-backend): Clean up internal macros (#71061)

Figured it was easier to fix these myself than to try to suggest changes on #70798: - Fix some inconsistency inside `iter_many` about how derefing happens depending on how arguments are used. - Simplify `get_many`'s argument passthrough to `iter_many` using a `tt` group: https://veykril.github.io/tlborm/decl-macros/patterns/tt-bundling.html - Manually line-wrap macros, which rustfmt does not do. - Rename some of `iter_many` arguments (the name "`value`" was a little too overloaded here) and add some really basic documentation. - Eliminate a few branches using `?` repetitions instead: https://veykril.github.io/tlborm/decl-macros/macros-methodical.html#repetitions - The `value_pattern` (previously `value_ident`) of `iter_many` can technically be any pattern (even though nothing currently needs that), so broaden the type from `ident` to `tt`.
vercel · Oct 11, 2024 · f613378 · f613378
1 parent 4ffdfe9
commit f613378
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 52 deletions.
diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs
@@ -1313,14 +1313,26 @@ impl TurboTasksBackendInner {
                 );
                 task = ctx.task(task_id, TaskDataCategory::All);
             }
-            for collectible in iter_many!(task, AggregatedCollectible { collectible } count if collectible.collectible_type == collectible_type && count > 0 => collectible.cell)
-            {
+            for collectible in iter_many!(
+                task,
+                AggregatedCollectible {
+                    collectible
+                } count if collectible.collectible_type == collectible_type && count > 0 => {
+                    collectible.cell
+                }
+            ) {
                 *collectibles
                     .entry(RawVc::TaskCell(collectible.task, collectible.cell))
                     .or_insert(0) += 1;
             }
-            for (collectible, count) in iter_many!(task, Collectible { collectible } count if collectible.collectible_type == collectible_type => (collectible.cell, count))
-            {
+            for (collectible, count) in iter_many!(
+                task,
+                Collectible {
+                    collectible
+                } count if collectible.collectible_type == collectible_type => {
+                    (collectible.cell, count)
+                }
+            ) {
                 *collectibles
                     .entry(RawVc::TaskCell(collectible.task, collectible.cell))
                     .or_insert(0) += count;

diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs
@@ -133,8 +133,14 @@ impl AggregatedDataUpdate {
             if dirty_container_count > 0 {
                 dirty = true;
             }
-            for collectible in iter_many!(task, AggregatedCollectible { collectible } count if count > 0 => collectible)
-            {
+            for collectible in iter_many!(
+                task,
+                AggregatedCollectible {
+                    collectible
+                } count if count > 0 => {
+                    collectible
+                }
+            ) {
                 collectibles_update.push((collectible, 1));
             }
         }
@@ -249,7 +255,15 @@ impl AggregatedDataUpdate {
             );
             if added || removed {
                 let ty = collectible.collectible_type;
-                let dependent: SmallVec<[TaskId; 4]> = get_many!(task, CollectiblesDependent { collectible_type, task } if *collectible_type == ty => *task);
+                let dependent: SmallVec<[TaskId; 4]> = get_many!(
+                    task,
+                    CollectiblesDependent {
+                        collectible_type,
+                        task,
+                    } if collectible_type == ty => {
+                        task
+                    }
+                );
                 if !dependent.is_empty() {
                     queue.push(AggregationUpdateJob::Invalidate {
                         task_ids: dependent,

diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs b/turbopack/crates/turbo-tasks-backend/src/backend/storage.rs
@@ -396,7 +396,9 @@ where
 
 macro_rules! get {
     ($task:ident, $key:ident $input:tt) => {
-        if let Some($crate::data::CachedDataItemValue::$key { value }) = $task.get(&$crate::data::CachedDataItemKey::$key $input).as_ref() {
+        if let Some($crate::data::CachedDataItemValue::$key {
+            value,
+        }) = $task.get(&$crate::data::CachedDataItemKey::$key $input).as_ref() {
             Some(value)
         } else {
             None
@@ -409,7 +411,9 @@ macro_rules! get {
 
 macro_rules! get_mut {
     ($task:ident, $key:ident $input:tt) => {
-        if let Some($crate::data::CachedDataItemValue::$key { value }) = $task.get_mut(&$crate::data::CachedDataItemKey::$key $input).as_mut() {
+        if let Some($crate::data::CachedDataItemValue::$key {
+            value,
+        }) = $task.get_mut(&$crate::data::CachedDataItemKey::$key $input).as_mut() {
             let () = $crate::data::allow_mut_access::$key;
             Some(value)
         } else {
@@ -421,64 +425,42 @@ macro_rules! get_mut {
     };
 }
 
+/// Creates an iterator over all [`CachedDataItemKey::$key`][crate::data::CachedDataItemKey]s in
+/// `$task` matching the given `$key_pattern`, optional `$value_pattern`, and optional `if $cond`.
+///
+/// Each element in the iterator is determined by `$iter_item`, which may use fields extracted by
+/// `$key_pattern` or `$value_pattern`.
 macro_rules! iter_many {
-    ($task:ident, $key:ident $input:tt => $value:expr) => {
-        $task
-            .iter($crate::data::indicies::$key)
-            .filter_map(|(key, _)| match *key {
-                $crate::data::CachedDataItemKey::$key $input => Some($value),
-                _ => None,
-            })
-    };
-    ($task:ident, $key:ident $input:tt => $value:expr) => {
-        $task
-            .iter($crate::data::indicies::$key)
-            .filter_map(|(key, _)| match key {
-                $crate::data::CachedDataItemKey::$key $input => Some($value),
-                _ => None,
-            })
-    };
-    ($task:ident, $key:ident $input:tt if $cond:expr => $value:expr) => {
+    ($task:ident, $key:ident $key_pattern:tt $(if $cond:expr)? => $iter_item:expr) => {
         $task
             .iter($crate::data::indicies::$key)
             .filter_map(|(key, _)| match key {
-                $crate::data::CachedDataItemKey::$key $input if $cond => Some($value),
+                &$crate::data::CachedDataItemKey::$key $key_pattern $(if $cond)? => Some(
+                    $iter_item
+                ),
                 _ => None,
             })
     };
-    ($task:ident, $key:ident $input:tt $value_ident:ident => $value:expr) => {
+    ($task:ident, $key:ident $input:tt $value_pattern:tt $(if $cond:expr)? => $iter_item:expr) => {
         $task
             .iter($crate::data::indicies::$key)
             .filter_map(|(key, value)| match (key, value) {
-                (&$crate::data::CachedDataItemKey::$key $input, &$crate::data::CachedDataItemValue::$key { value: $value_ident }) => Some($value),
-                _ => None,
-            })
-    };
-    ($task:ident, $key:ident $input:tt $value_ident:ident if $cond:expr => $value:expr) => {
-        $task
-            .iter($crate::data::indicies::$key)
-            .filter_map(|(key, value)| match (key, value) {
-                (&$crate::data::CachedDataItemKey::$key $input, &$crate::data::CachedDataItemValue::$key { value: $value_ident }) if $cond => Some($value),
+                (
+                    &$crate::data::CachedDataItemKey::$key $input,
+                    &$crate::data::CachedDataItemValue::$key { value: $value_pattern }
+                ) $(if $cond)? => Some($iter_item),
                 _ => None,
             })
     };
 }
 
+/// A thin wrapper around [`iter_many`] that calls [`Iterator::collect`].
+///
+/// Note that the return type of [`Iterator::collect`] may be ambiguous in certain contexts, so
+/// using this macro may require explicit type annotations on variables.
 macro_rules! get_many {
-    ($task:ident, $key:ident $input:tt => $value:expr) => {
-        $crate::backend::storage::iter_many!($task, $key $input => $value).collect()
-    };
-    ($task:ident, $key:ident $input:tt => $value:expr) => {
-        $crate::backend::storage::iter_many!($task, $key $input => $value).collect()
-    };
-    ($task:ident, $key:ident $input:tt if $cond:expr => $value:expr) => {
-        $crate::backend::storage::iter_many!($task, $key $input if $cond => $value).collect()
-    };
-    ($task:ident, $key:ident $input:tt $value_ident:ident => $value:expr) => {
-        $crate::backend::storage::iter_many!($task, $key $input $value_ident => $value).collect()
-    };
-    ($task:ident, $key:ident $input:tt $value_ident:ident if $cond:expr => $value:expr) => {
-        $crate::backend::storage::iter_many!($task, $key $input $value_ident if $cond => $value).collect()
+    ($($args:tt)*) => {
+        $crate::backend::storage::iter_many!($($args)*).collect()
     };
 }
 
@@ -529,7 +511,9 @@ macro_rules! update_count {
 
 macro_rules! remove {
     ($task:ident, $key:ident $input:tt) => {
-        if let Some($crate::data::CachedDataItemValue::$key { value }) = $task.remove(&$crate::data::CachedDataItemKey::$key $input) {
+        if let Some($crate::data::CachedDataItemValue::$key { value }) = $task.remove(
+            &$crate::data::CachedDataItemKey::$key $input
+        ) {
             Some(value)
         } else {
             None