Use enums to clarify DepNodeColorMap color marking

When a function's documentation has to explain the meaning of nested results
and options, then it is often a good candidate for using a custom result enum
instead.

This commit also renames `DepNodeColorMap::try_mark` to `try_set_color`, to
make it more distinct from the similarly-named `DepGraph::try_mark_green`.

The difference is that `try_mark_green` is a higher-level operation that tries
to determine whether a node _can_ be marked green, whereas `try_set_color` is a
lower-level operation that actually records a color for the node.

compiler/rustc_middle/src/dep_graph/graph.rs

@@ -1415,28 +1415,29 @@ pub(super) fn current(&self, index: SerializedDepNodeIndex) -> Option<DepNodeInd
         if value <= DepNodeIndex::MAX_AS_U32 { Some(DepNodeIndex::from_u32(value)) } else { None }
     }
 
-    /// This tries to atomically mark a node green and assign `index` as the new
+    /// Atomically sets the color of a previous-session dep node to either green
-    /// index if `green` is true, otherwise it will try to atomicaly mark it red.
+    /// or red, if it has not already been colored.
     ///
-    /// This returns `Ok` if `index` gets assigned or the node is marked red, otherwise it returns
+    /// If the node already has a color, the new color is ignored, and the
-    /// the already allocated index in `Err` if it is green already. If it was already
+    /// return value indicates the existing color.
-    /// red, `Err(None)` is returned.
     #[inline(always)]
-    pub(super) fn try_mark(
+    pub(super) fn try_set_color(
         &self,
         prev_index: SerializedDepNodeIndex,
-        index: DepNodeIndex,
+        color: DesiredColor,
-        green: bool,
+    ) -> TrySetColorResult {
-    ) -> Result<(), Option<DepNodeIndex>> {
+        match self.values[prev_index].compare_exchange(
-        let value = &self.values[prev_index];
-        match value.compare_exchange(
             COMPRESSED_UNKNOWN,
-            if green { index.as_u32() } else { COMPRESSED_RED },
+            match color {
+                DesiredColor::Red => COMPRESSED_RED,
+                DesiredColor::Green { index } => index.as_u32(),
+            },
             Ordering::Relaxed,
             Ordering::Relaxed,
         ) {
-            Ok(_) => Ok(()),
+            Ok(_) => TrySetColorResult::Success,
-            Err(v) => Err(if v == COMPRESSED_RED { None } else { Some(DepNodeIndex::from_u32(v)) }),
+            Err(COMPRESSED_RED) => TrySetColorResult::AlreadyRed,
+            Err(index) => TrySetColorResult::AlreadyGreen { index: DepNodeIndex::from_u32(index) },
         }
     }
 
@@ -1463,6 +1464,28 @@ pub(super) fn insert_red(&self, index: SerializedDepNodeIndex) {
     }
 }
 
+/// The color that [`DepNodeColorMap::try_set_color`] should try to apply to a node.
+#[derive(Clone, Copy, Debug)]
+pub(super) enum DesiredColor {
+    /// Try to mark the node red.
+    Red,
+    /// Try to mark the node green, associating it with a current-session node index.
+    Green { index: DepNodeIndex },
+}
+
+/// Return value of [`DepNodeColorMap::try_set_color`], indicating success or failure,
+/// and (on failure) what the existing color is.
+#[derive(Clone, Copy, Debug)]
+pub(super) enum TrySetColorResult {
+    /// The [`DesiredColor`] was freshly applied to the node.
+    Success,
+    /// Coloring failed because the node was already marked red.
+    AlreadyRed,
+    /// Coloring failed because the node was already marked green,
+    /// and corresponds to node `index` in the current-session dep graph.
+    AlreadyGreen { index: DepNodeIndex },
+}
+
 #[inline(never)]
 #[cold]
 pub(crate) fn print_markframe_trace(graph: &DepGraph, frame: &MarkFrame<'_>) {

compiler/rustc_middle/src/dep_graph/serialized.rs

@@ -58,7 +58,7 @@
 use rustc_session::Session;
 use tracing::{debug, instrument};
 
-use super::graph::{CurrentDepGraph, DepNodeColorMap};
+use super::graph::{CurrentDepGraph, DepNodeColorMap, DesiredColor, TrySetColorResult};
 use super::retained::RetainedDepGraph;
 use super::{DepKind, DepNode, DepNodeIndex};
 use crate::dep_graph::edges::EdgesVec;
@@ -905,13 +905,14 @@ pub(crate) fn send_and_color(
         let mut local = self.status.local.borrow_mut();
 
         let index = self.status.next_index(&mut *local);
+        let color = if is_green { DesiredColor::Green { index } } else { DesiredColor::Red };
 
-        // Use `try_mark` to avoid racing when `send_promoted` is called concurrently
+        // Use `try_set_color` to avoid racing when `send_promoted` is called concurrently
         // on the same index.
-        match colors.try_mark(prev_index, index, is_green) {
+        match colors.try_set_color(prev_index, color) {
-            Ok(()) => (),
+            TrySetColorResult::Success => {}
-            Err(None) => panic!("dep node {:?} is unexpectedly red", prev_index),
+            TrySetColorResult::AlreadyRed => panic!("dep node {prev_index:?} is unexpectedly red"),
-            Err(Some(dep_node_index)) => return dep_node_index,
+            TrySetColorResult::AlreadyGreen { index } => return index,
         }
 
         self.status.bump_index(&mut *local);
@@ -923,7 +924,8 @@ pub(crate) fn send_and_color(
     /// from the previous dep graph and expects all edges to already have a new dep node index
     /// assigned.
     ///
-    /// This will also ensure the dep node is marked green if `Some` is returned.
+    /// Tries to mark the dep node green, and returns Some if it is now green,
+    /// or None if had already been concurrently marked red.
     #[inline]
     pub(crate) fn send_promoted(
         &self,
@@ -935,10 +937,10 @@ pub(crate) fn send_promoted(
         let mut local = self.status.local.borrow_mut();
         let index = self.status.next_index(&mut *local);
 
-        // Use `try_mark_green` to avoid racing when `send_promoted` or `send_and_color`
+        // Use `try_set_color` to avoid racing when `send_promoted` or `send_and_color`
         // is called concurrently on the same index.
-        match colors.try_mark(prev_index, index, true) {
+        match colors.try_set_color(prev_index, DesiredColor::Green { index }) {
-            Ok(()) => {
+            TrySetColorResult::Success => {
                 self.status.bump_index(&mut *local);
                 self.status.encode_promoted_node(
                     index,
@@ -949,7 +951,8 @@ pub(crate) fn send_promoted(
                 );
                 Some(index)
             }
-            Err(dep_node_index) => dep_node_index,
+            TrySetColorResult::AlreadyRed => None,
+            TrySetColorResult::AlreadyGreen { index } => Some(index),
         }
     }
 

typos.toml

@@ -48,6 +48,7 @@ unstalled = "unstalled" # short for un-stalled
 # the non-empty form can be automatically fixed by `--bless`.
 #
 # tidy-alphabetical-start
+atomicaly = "atomically"
 definitinon = "definition"
 dependy = ""
 similarlty = "similarity"