diff --git a/src/archive/find.rs b/src/archive/find.rs index 083d4a97..3f176eba 100644 --- a/src/archive/find.rs +++ b/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; } diff --git a/src/metadata/metadata_document.rs b/src/metadata/metadata_document.rs index 25f1dd4d..6549d17b 100644 --- a/src/metadata/metadata_document.rs +++ b/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) { diff --git a/src/sorting/dfs.rs b/src/sorting/dfs.rs index 0484756c..42999941 100644 --- a/src/sorting/dfs.rs +++ b/src/sorting/dfs.rs @@ -149,8 +149,9 @@ fn edges( // 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() diff --git a/src/sorting/groups.rs b/src/sorting/groups.rs index 25f5c992..28d5ce5a 100644 --- a/src/sorting/groups.rs +++ b/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(), diff --git a/src/sorting/plugins.rs b/src/sorting/plugins.rs index 800d8ec4..d38231be 100644 --- a/src/sorting/plugins.rs +++ b/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>, 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>, } @@ -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 [{}]", diff --git a/src/sorting/validate.rs b/src/sorting/validate.rs index 5e74df8c..b8c54883 100644 --- a/src/sorting/validate.rs +++ b/src/sorting/validate.rs @@ -49,8 +49,6 @@ pub fn validate_specific_and_hardcoded_edges( 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(())