Use a consistent prefix for comments about ideally-unnecessary code

Some of these limitations could be overcome with new std APIs, unsafe code, or language features, while it might be possible to overcome a few by taking a different approach.

src/archive/find.rs

@@ -106,7 +106,10 @@ fn find_associated_archives_with_arbitrary_suffixes(
                 return false;
             };
 
-            // Can't just slice the archive filename to the same length as the plugin file stem directly because that might not slice on a character boundary, so truncate the byte slice and then check it's still valid UTF-8.
+            // Can't just slice the archive filename to the same length as the
+            // plugin file stem directly because that might not slice on a
+            // character boundary, so truncate the byte slice and then check
+            // it's still valid UTF-8.
             if archive_filename.len() < plugin_stem_len {
                 return false;
             }

src/metadata/metadata_document.rs

@@ -357,8 +357,10 @@ fn split_on_prelude(masterlist: &str) -> Option<(&str, &str)> {
 
         if let Some((_, next_byte)) = iter.peek() {
             if !matches!(next_byte, b' ' | b'#' | b'\n' | b'\r') {
-                // Slicing at index should never fail, but we can't prove that,
-                // and we don't want to risk panicking.
+                // LIMITATION: Slicing at index should never fail, but the
+                // compiler can't see that. A variation of str.find() that
+                // could take a closure that matches on substrings would
+                // eliminate the need for this.
                 if let Some(suffix) = remainder.get(index..) {
                     return Some((prefix, suffix));
                 }
@@ -378,8 +380,8 @@ fn split_on_prelude_start(masterlist: &str) -> Option<(&str, &str)> {
     } else {
         if let Some(pos) = masterlist.find(prelude_on_new_line) {
             let index = pos + prelude_on_new_line.len();
-            // A checked split shouldn't be necessary, but there's no
-            // split_inclusive_once() method, so we need to find and split in
+            // LIMITATION: A checked split shouldn't be necessary, but there's
+            // no split_inclusive_once() method, so we need to find and split in
             // two steps and there's always the risk of a bug being introduced
             // in the middle.
             if let Some((prefix, remainder)) = masterlist.split_at_checked(index) {

src/sorting/dfs.rs

@@ -149,8 +149,9 @@ fn edges<N>(
     // Petgraph produces edges in the reverse of the order that neighbouring
     // nodes were added to the graph, but for backwards compatibility we want
     // the opposite order.
-    // Unfortunately Petgraph's Edges iterator doesn't impl DoubleEndedIterator
-    // (though it probably could), so this needs to buffer the edges.
+    // LIMITATION: Unfortunately Petgraph's Edges iterator doesn't impl
+    // DoubleEndedIterator (though it probably could), so this needs to buffer
+    // the edges.
     let mut edges: Vec<_> = graph.edges(node_index).collect();
     edges.reverse();
     edges.into_iter()

src/sorting/groups.rs

@@ -87,6 +87,8 @@ fn add_groups<'a>(
         }
 
         let Some(node_index) = group_nodes.get(group.name()) else {
+            // LIMITATION: This should be impossible, as all the groups have
+            // just been added in the previous for loop.
             logging::error!(
                 "Unexpectedly couldn't find node for group {}: it should have just been added to the graph",
                 group.name()
@@ -151,6 +153,8 @@ pub fn find_path(
                 return Ok(Vec::new());
             }
             _ => {
+                // LIMITATION: This should be impossible, the predecessors array
+                // should have an index for every node in the graph.
                 return Err(PathfindingError::PrecedingNodeNotFound(
                     graph[current].clone().into_string(),
                 )
@@ -168,6 +172,8 @@ pub fn find_path(
         }
 
         let Some(edge) = graph.find_edge(*preceding_vertex, current) else {
+            // LIMITATION: This should be impossible, bellman_ford says there's
+            // an edge.
             return Err(PathfindingError::EdgeNotFound {
                 from_group: graph[*preceding_vertex].clone().into_string(),
                 to_group: graph[current].clone().into_string(),

src/sorting/plugins.rs

@@ -145,11 +145,12 @@ fn to_filenames(files: &[File]) -> Box<[String]> {
     files.iter().map(|f| f.name().as_str().to_owned()).collect()
 }
 
+// LIMITATION: Use Rc so that sorting can add edges to the graph while holding
+// references to plugin sorting data.
 type InnerPluginsGraph<'a, T> = Graph<Rc<PluginSortingData<'a, T>>, EdgeType>;
 
 #[derive(Debug)]
 struct PluginsGraph<'a, T: SortingPlugin> {
-    // Put the sorting data in Rc so that it can be held onto while mutating the graph.
     inner: InnerPluginsGraph<'a, T>,
     paths_cache: HashMap<NodeIndex, HashSet<NodeIndex>>,
 }
@@ -272,6 +273,9 @@ impl<'a, T: SortingPlugin> PluginsGraph<'a, T> {
         early_loader_indices.sort_by_key(|e| e.0);
 
         for window in early_loader_indices.windows(2) {
+            // LIMITATION: This should be infallible, the windows are of fixed
+            // size. The array_windows function would solve this, but it's
+            // unstable.
             if let [(_, from_index), (_, to_index)] = *window {
                 self.add_edge(from_index, to_index, EdgeType::Hardcoded);
             }
@@ -504,7 +508,9 @@ impl<'a, T: SortingPlugin> PluginsGraph<'a, T> {
 
         for window in nodes.windows(2) {
             let [current, next] = *window else {
-                // This should never happen.
+            // LIMITATION: This should be impossible, the windows are of fixed
+            // size. The array_windows function would solve this, but it's
+            // unstable.
                 logging::error!("Unexpectedly encountered a window length that was not 2");
                 continue;
             };
@@ -700,6 +706,9 @@ impl<'a, T: SortingPlugin> PluginsGraph<'a, T> {
 
         path.windows(2).find_map(|slice| match *slice {
             [a, b] => self.inner.contains_edge(a, b).not().then_some((a, b)),
+            // LIMITATION: This should be impossible, the windows are of fixed
+            // size. The array_windows function would solve this, but it's
+            // unstable.
             _ => None,
         })
     }
@@ -917,6 +926,8 @@ impl<'a, 'b, T: SortingPlugin> PathFinder<'a, 'b, T> {
                         path.push(*next);
                         current_node = *next;
                     } else {
+                        // LIMITATION: This should be impossible, the existence
+                        // of an intersection node indicates that a path exists.
                         logging::error!(
                             "Could not find parent vertex of {}. Path so far is {}",
                             self.graph[current_node].name(),
@@ -938,6 +949,8 @@ impl<'a, 'b, T: SortingPlugin> PathFinder<'a, 'b, T> {
                         path.push(*next);
                         current_node = *next;
                     } else {
+                        // LIMITATION: This should be impossible, the existence
+                        // of an intersection node indicates that a path exists.
                         logging::error!(
                             "Could not find child vertex of {}. Path so far is {}",
                             self.graph[current_node].name(),
@@ -1092,6 +1105,8 @@ impl<'a, 'b, 'c, 'd, 'e, T: SortingPlugin> GroupsPathVisitor<'a, 'b, 'c, 'd, 'e,
         use std::fmt::Write;
 
         let Some([from_edge, edges @ ..]) = self.edge_stack.get(edge_stack_index..) else {
+            // LIMITATION: The index should be valid, as it's only used to avoid
+            // borrowing the edge stack while adding edges.
             if is_log_enabled(LogLevel::Error) {
                 logging::error!(
                     "Unexpected invalid edge stack index {} for edge stack [{}]",

src/sorting/validate.rs

@@ -49,8 +49,6 @@ pub fn validate_specific_and_hardcoded_edges<T: SortingPlugin>(
 
     validate_non_masters(non_masters, &blueprint_masters_set)?;
 
-    // There's at least one master, check that there are no hardcoded
-    // non-masters.
     validate_early_loading_plugins(early_loading_plugins, masters, &non_masters_set)?;
 
     Ok(())