From 63d10426677a13cd5223a4418821b35a339fec6b Mon Sep 17 00:00:00 2001 From: "Tom D." <15268361+anastygnome@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:40:38 +0200 Subject: [PATCH 1/2] tsort: refactor parser and make its allocations safe --- src/uu/tsort/src/parser.rs | 82 +++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/src/uu/tsort/src/parser.rs b/src/uu/tsort/src/parser.rs index a14652a8fb..11ab46c138 100644 --- a/src/uu/tsort/src/parser.rs +++ b/src/uu/tsort/src/parser.rs @@ -2,67 +2,65 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use memchr::memchr3; +use memchr::memchr3_iter; use std::io::{self, BufRead}; +#[inline(always)] +/// Reads whitespace-separated tokens and passes each token to `f`. +/// +/// The input is processed a buffer at a time because `tsort` reads an +/// unbounded stream of whitespace-separated tokens. pub fn for_each_token(mut reader: R, mut f: F) -> io::Result<()> where R: BufRead, F: FnMut(&[u8]), { + // Holds a partial token when the current buffer ends before its delimiter. let mut pending = Vec::new(); loop { - let buf = reader.fill_buf()?; - - if buf.is_empty() { - if !pending.is_empty() { - f(&pending); - } - return Ok(()); - } - - let mut pos = 0; - - while pos < buf.len() { - if pending.is_empty() { - // Skip whitespace before the next token. - while pos < buf.len() && is_delimiter(buf[pos]) { - pos += 1; - } - - if pos == buf.len() { - break; + // Keep the borrow scoped for later call to consume. + let consumed = { + let buf = reader.fill_buf()?; + if buf.is_empty() { + // EOF => process any pending token. + if !pending.is_empty() { + f(&pending); } + return Ok(()); } - if let Some(i) = memchr3(b' ', b'\t', b'\n', &buf[pos..]) { - let end = pos + i; + let mut start = 0; + // Find each delimiter in this Buf chunk. The bytes between `start` and + // a delimiter form one complete token. + for end in memchr3_iter(b' ', b'\t', b'\n', buf) { + if !pending.is_empty() { + // This token started in the previous chunk and ends here. + // try_reserve first to report the allocation failure. + pending.try_reserve(end - start)?; + pending.extend_from_slice(&buf[start..end]); - if pending.is_empty() { - // Fast path: token is entirely inside this buffer. - f(&buf[pos..end]); - } else { - // Complete a token that started in an earlier buffer. - pending.extend_from_slice(&buf[pos..end]); f(&pending); pending.clear(); - } + } else if start != end { + // The token is entirely in this chunk, so avoid copying it. + f(&buf[start..end]); + } // else we encountered several whitespaces, keep going + + // move past the separator to the next token. + start = end + 1; + } - pos = end + 1; - } else { - // Token continues into the next fill_buf() chunk. - pending.extend_from_slice(&buf[pos..]); - break; + if start != buf.len() { + // last token of the input (has no space after it). + pending.try_reserve(buf.len() - start)?; + pending.extend_from_slice(&buf[start..]); } - } - let consumed = buf.len(); + // end borrow for consumption. + buf.len() + }; + reader.consume(consumed); } } - -#[inline] -fn is_delimiter(byte: u8) -> bool { - matches!(byte, b' ' | b'\t' | b'\n') -} From fe099748f98490a325b0ec5871bd0c16e286438b Mon Sep 17 00:00:00 2001 From: "Tom D." <15268361+anastygnome@users.noreply.github.com> Date: Sun, 13 Sep 2026 10:18:52 +0200 Subject: [PATCH 2/2] fixup! tsort: refactor parser and make its allocations safe --- src/uu/tsort/src/error.rs | 12 ++ src/uu/tsort/src/graph.rs | 356 +++++++++++++++++++++++++++++++++++ src/uu/tsort/src/interner.rs | 131 +++++++------ src/uu/tsort/src/parser.rs | 45 +++-- src/uu/tsort/src/tsort.rs | 325 ++++---------------------------- 5 files changed, 491 insertions(+), 378 deletions(-) create mode 100644 src/uu/tsort/src/graph.rs diff --git a/src/uu/tsort/src/error.rs b/src/uu/tsort/src/error.rs index 186bc35e40..c1917eaeea 100644 --- a/src/uu/tsort/src/error.rs +++ b/src/uu/tsort/src/error.rs @@ -3,10 +3,17 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +use std::collections::TryReserveError as StdTryReserveError; use std::io; use uucore::{display::Quotable as _, translate}; +#[derive(Debug, thiserror::Error)] +pub(crate) enum AllocationError { + #[error("{0}")] + Std(#[from] StdTryReserveError), +} + #[derive(Debug, thiserror::Error)] pub(crate) enum Error { /// Error while reading input. @@ -17,6 +24,11 @@ pub(crate) enum Error { #[error("{message}: {0}", message = translate!("common-write-error"))] Write(io::Error), + /// A collection could not grow because its capacity overflowed or the + /// allocator rejected the request. + #[error(transparent)] + Allocation(#[from] AllocationError), + /// The graph contains a cycle. #[error("{input}: {message}", input = .0, message = translate!("tsort-error-loop"))] Loop(String), diff --git a/src/uu/tsort/src/graph.rs b/src/uu/tsort/src/graph.rs new file mode 100644 index 0000000000..08caf8cac5 --- /dev/null +++ b/src/uu/tsort/src/graph.rs @@ -0,0 +1,356 @@ +// This file is part of the uutils coreutils package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +// spell-checker:ignore TAOCP indegree + +use rustc_hash::FxHashMap; +use std::collections::VecDeque; +use std::collections::hash_map::Entry; +use std::fmt; +use std::io::{self, BufWriter, Write}; +use uucore::error::UError; +use uucore::show; + +use crate::error::{AllocationError, Error}; +use crate::interner::{ByteInterner, ByteInternerBuilder, Sym}; +use crate::try_clone_str; + +// Auxiliary struct, just for printing loop nodes via show! macro. +// +// Diagnostics go through Display, so invalid UTF-8 bytes are represented +// lossily here. +#[derive(Debug)] +struct LoopNode<'a>(&'a [u8]); + +impl fmt::Display for LoopNode<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", String::from_utf8_lossy(self.0)) + } +} + +impl std::error::Error for LoopNode<'_> {} +impl UError for LoopNode<'_> {} + +/// Find the element `x` in `vec` and remove it, returning its index. +fn remove(vec: &mut Vec, x: T) -> Option +where + T: PartialEq, +{ + vec.iter().position(|item| *item == x).inspect(|i| { + vec.remove(*i); + }) +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum VisitedState { + Opened, + Closed, +} + +#[derive(Default)] +struct Node { + successor_tokens: Vec, + predecessor_count: usize, +} + +impl Node { + fn add_successor(&mut self, successor_name: Sym) -> Result<(), AllocationError> { + self.successor_tokens.try_reserve(1)?; + self.successor_tokens.push(successor_name); + Ok(()) + } +} + +pub(crate) struct GraphBuilder { + name: String, + nodes: FxHashMap, + interner: ByteInternerBuilder, +} + +impl GraphBuilder { + pub(crate) fn new(name: String) -> Self { + Self { + name, + nodes: FxHashMap::default(), + interner: ByteInternerBuilder::default(), + } + } + + pub(crate) fn name(&self) -> &str { + &self.name + } + + #[inline] + pub(crate) fn intern(&mut self, value: &[u8]) -> Result { + self.interner.get_or_intern(value) + } + + pub(crate) fn add_edge(&mut self, from: Sym, to: Sym) -> Result<(), AllocationError> { + // Reserve only for keys that are actually absent. This avoids turning an + // edge between existing nodes into a spurious capacity-overflow failure. + let missing_from = !self.nodes.contains_key(&from); + let missing_to = from != to && !self.nodes.contains_key(&to); + let additional = usize::from(missing_from) + usize::from(missing_to); + self.nodes.try_reserve(additional)?; + + let from_node = self.nodes.entry(from).or_default(); + + if from != to { + from_node.add_successor(to)?; + + let to_node = self.nodes.entry(to).or_default(); + to_node.predecessor_count += 1; + } + + Ok(()) + } + + pub(crate) fn finish(self) -> Graph { + Graph { + name: self.name, + nodes: self.nodes, + interner: self.interner.finish(), + } + } +} + +pub(crate) struct Graph { + name: String, + nodes: FxHashMap, + interner: ByteInterner, +} + +impl Graph { + fn name(&self) -> &str { + &self.name + } + + fn get_node_name(&self, node_sym: Sym) -> &[u8] { + self.interner + .resolve(node_sym) + .expect("symbol should be interned") + } + + fn remove_edge(&mut self, u: Sym, v: Sym) { + remove( + &mut self + .nodes + .get_mut(&u) + .expect("node is part of the graph") + .successor_tokens, + v, + ); + + self.nodes + .get_mut(&v) + .expect("node is part of the graph") + .predecessor_count -= 1; + } + + /// Implementation of algorithm T from TAOCP (Don. Knuth), vol. 1. + pub(crate) fn run_tsort(&mut self) -> Result<(), Error> { + // A node is enqueued at most once, so reserving the number of graph nodes + // covers the queue for the whole sort. + let mut independent_nodes_queue = VecDeque::new(); + independent_nodes_queue + .try_reserve(self.nodes.len()) + .map_err(AllocationError::from)?; + + for (&sym, node) in &self.nodes { + if node.predecessor_count == 0 { + independent_nodes_queue.push_back(sym); + } + } + + // Sort by name for deterministic output. + independent_nodes_queue + .make_contiguous() + .sort_unstable_by(|a, b| self.get_node_name(*a).cmp(self.get_node_name(*b))); + + let mut out = BufWriter::new(io::stdout().lock()); + + while !self.nodes.is_empty() { + let v = self.find_next_node(&mut independent_nodes_queue)?; + + // Write the node exactly as it appeared in the input, followed by a newline + out.write_all(self.get_node_name(v)).map_err(Error::Write)?; + writeln!(out).map_err(Error::Write)?; + + if let Some(node_to_process) = self.nodes.remove(&v) { + for successor_name in node_to_process.successor_tokens.into_iter().rev() { + // we reverse to match GNU tsort order + let successor_node = self + .nodes + .get_mut(&successor_name) + .expect("node is part of the graph"); + + successor_node.predecessor_count -= 1; + + if successor_node.predecessor_count == 0 { + // The queue was reserved for every graph node above, and + // each node reaches in-degree zero at most once. + independent_nodes_queue.push_back(successor_name); + } + } + } + } + + out.flush().map_err(Error::Write)?; + Ok(()) + } + + fn indegree(&self, sym: Sym) -> Option { + self.nodes.get(&sym).map(|data| data.predecessor_count) + } + + fn find_next_node(&mut self, frontier: &mut VecDeque) -> Result { + // If there are no nodes of in-degree zero but there are still + // un-visited nodes in the graph, then there must be a cycle. + // We need to find the cycle, display it on stderr, and break it to go on. + // + // A cycle is guaranteed to be of length at least two. We break + // the cycle by deleting an arbitrary edge (the first). That is + // not necessarily the optimal thing, but it should be enough to + // continue making progress in the graph traversal, and matches GNU tsort behavior. + // + // It is possible that deleting the edge does not actually + // result in the target node having in-degree zero, so we repeat + // the process until such a node appears. + + loop { + match frontier.pop_front() { + None => self.find_and_break_cycle(frontier)?, + Some(v) => return Ok(v), + } + } + } + + fn find_and_break_cycle(&mut self, frontier: &mut VecDeque) -> Result<(), Error> { + let cycle = self.detect_cycle()?; + + show!(Error::Loop(try_clone_str(self.name())?)); + + for &sym in &cycle { + show!(LoopNode(self.get_node_name(sym))); + } + + let u = *cycle.last().expect("cycle must be non-empty"); + let v = cycle[0]; + + self.remove_edge(u, v); + + if self.indegree(v).expect("node is part of the graph") == 0 { + // `frontier` was reserved for all nodes in run_tsort(). + frontier.push_back(v); + } + + Ok(()) + } + + fn detect_cycle(&self) -> Result, Error> { + // All three work structures are bounded by the number of remaining + // graph nodes, which is known at this point. + let node_count = self.nodes.len(); + + let mut nodes = Vec::new(); + nodes + .try_reserve(node_count) + .map_err(AllocationError::from)?; + for &sym in self.nodes.keys() { + nodes.push(sym); + } + + // Sort by name for deterministic output. + nodes.sort_unstable_by(|a, b| self.get_node_name(*a).cmp(self.get_node_name(*b))); + + let mut visited = FxHashMap::default(); + visited + .try_reserve(node_count) + .map_err(AllocationError::from)?; + + let mut stack = Vec::new(); + stack + .try_reserve(node_count) + .map_err(AllocationError::from)?; + + for &node in &nodes { + if let Some(loop_entry) = self.dfs(node, &mut visited, &mut stack) { + let start = stack + .iter() + .rposition(|(node, _)| *node == loop_entry) + .expect("loop entry must be on the DFS stack"); + + let mut cycle = Vec::new(); + cycle + .try_reserve(stack.len() - start) + .map_err(AllocationError::from)?; + + for &(node, _) in &stack[start..] { + cycle.push(node); + } + + return Ok(cycle); + } + } + + unreachable!("detect_cycle is expected to be called only on graphs with cycles"); + } + + fn dfs<'a>( + &'a self, + node: Sym, + visited: &mut FxHashMap, + stack: &mut Vec<(Sym, &'a [Sym])>, + ) -> Option { + stack.push(( + node, + self.nodes + .get(&node) + .map_or(&[], |n: &Node| &n.successor_tokens), + )); + + let state = *visited.entry(node).or_insert(VisitedState::Opened); + + if state == VisitedState::Closed { + // Remove the frame we just added. Keeping closed frames around makes + // the stack larger and complicates extracting the actual cycle. + stack.pop(); + return None; + } + + while let Some((node, pending_successors)) = stack.pop() { + let Some((&next_node, pending)) = pending_successors.split_first() else { + // no more pending successors in the list -> close the node + visited.insert(node, VisitedState::Closed); + continue; + }; + + // schedule processing for the pending part of successors for this node + stack.push((node, pending)); + + match visited.entry(next_node) { + Entry::Vacant(v) => { + // first visit of the node + v.insert(VisitedState::Opened); + + stack.push(( + next_node, + self.nodes + .get(&next_node) + .map_or(&[], |n| &n.successor_tokens), + )); + } + + Entry::Occupied(o) => { + if *o.get() == VisitedState::Opened { + return Some(next_node); + } + } + } + } + + None + } +} diff --git a/src/uu/tsort/src/interner.rs b/src/uu/tsort/src/interner.rs index 375194d05b..117a831e4b 100644 --- a/src/uu/tsort/src/interner.rs +++ b/src/uu/tsort/src/interner.rs @@ -3,51 +3,46 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use hashbrown::HashTable; +use crate::error::AllocationError; +use hashbrown::{HashTable, hash_table::Entry}; use rustc_hash::FxHasher; use std::hash::Hasher; pub type Sym = usize; -/// Interns arbitrary byte strings. +/// Mutable byte-string interning phase. /// -/// During interning: +/// During construction: /// /// &[u8] -> Sym -/// Sym -> &[u8] /// -/// Once `finish_interning()` is called, the hash table used for -/// `&[u8] -> Sym` lookups is dropped. `Sym -> &[u8]` resolution remains -/// available for the lifetime of the interner. -pub struct ByteInterner { +/// Call [`finish`](Self::finish) after all values have been interned to drop +/// the lookup table and obtain a [`ByteInterner`] for `Sym -> &[u8]` +/// resolution. +#[derive(Default)] +pub struct ByteInternerBuilder { /// Needed only while new values are being interned. - table: Option>, + table: HashTable, /// All interned byte strings packed contiguously. bytes: Vec, - /// Boundaries into `bytes`. + /// End offsets into `bytes. /// /// Symbol `n` corresponds to: /// - /// bytes[offsets[n]..offsets[n + 1]] + /// bytes[ends[n-1]..ends[n]] /// - /// The initial zero is a sentinel, so the number of symbols is - /// `offsets.len() - 1`. - offsets: Vec, + ends: Vec, } -impl Default for ByteInterner { - fn default() -> Self { - Self { - table: Some(HashTable::new()), - bytes: Vec::new(), - offsets: vec![0], - } - } +/// Finished byte-string interner. +pub struct ByteInterner { + bytes: Vec, + ends: Vec, } -impl ByteInterner { +impl ByteInternerBuilder { #[inline] fn hash(value: &[u8]) -> u64 { let mut hasher = FxHasher::default(); @@ -56,60 +51,62 @@ impl ByteInterner { } #[inline] - pub fn get_or_intern(&mut self, value: &[u8]) -> Sym { - let hash = Self::hash(value); - - let Self { - table, - bytes, - offsets, - } = self; - - let table = table - .as_mut() - .expect("cannot intern values after finish_interning()"); + fn bounds(ends: &[usize], sym: Sym) -> (usize, usize) { + let end = ends[sym]; + let start = if sym == 0 { 0 } else { ends[sym - 1] }; + (start, end) + } - // Check whether this byte sequence is already interned. - if let Some(&sym) = table.find(hash, |&sym| { - let start = offsets[sym]; - let end = offsets[sym + 1]; + #[inline] + pub fn get_or_intern(&mut self, value: &[u8]) -> Result { + let hash = Self::hash(value); - &bytes[start..end] == value - }) { - return sym; + let Self { table, bytes, ends } = self; + + match table.entry( + hash, + |&sym| Self::value_for(bytes, ends, sym) == value, + |&sym| Self::hash(Self::value_for(bytes, ends, sym)), + ) { + Entry::Occupied(entry) => Ok(*entry.get()), + Entry::Vacant(entry) => { + bytes.try_reserve(value.len())?; + ends.try_reserve(1)?; + + let sym = ends.len(); + bytes.extend_from_slice(value); + ends.push(bytes.len()); + entry.insert(sym); + + Ok(sym) + } } + } - // Allocate a new symbol and append the bytes directly to the arena. - let sym = offsets.len() - 1; - - bytes.extend_from_slice(value); - offsets.push(bytes.len()); - - // HashTable stores only the symbol. If it needs to resize, hashes - // for existing entries are reconstructed from the byte arena. - table.insert_unique(hash, sym, |&sym| { - let start = offsets[sym]; - let end = offsets[sym + 1]; - - Self::hash(&bytes[start..end]) - }); + #[inline] + pub fn finish(self) -> ByteInterner { + ByteInterner { + bytes: self.bytes, + ends: self.ends, + } + } - sym + fn value_for<'a>(bytes: &'a [u8], ends: &[usize], sym: usize) -> &'a [u8] { + let (start, end) = Self::bounds(ends, sym); + &bytes[start..end] } +} +impl ByteInterner { #[inline] pub fn resolve(&self, sym: Sym) -> Option<&[u8]> { - let start = *self.offsets.get(sym)?; - let end = *self.offsets.get(sym + 1)?; + let end = *self.ends.get(sym)?; + let start = if sym == 0 { + 0 + } else { + *self.ends.get(sym - 1)? + }; Some(&self.bytes[start..end]) } - - /// Drop the `bytes -> Sym` lookup structure. - /// - /// Call this after all input has been parsed and no more values will - /// be interned. - pub fn finish_interning(&mut self) { - drop(self.table.take()); - } } diff --git a/src/uu/tsort/src/parser.rs b/src/uu/tsort/src/parser.rs index 11ab46c138..badd85a830 100644 --- a/src/uu/tsort/src/parser.rs +++ b/src/uu/tsort/src/parser.rs @@ -2,62 +2,71 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. + use memchr::memchr3_iter; -use std::io::{self, BufRead}; +use std::io::BufRead; + +use crate::error::{AllocationError, Error, ReadError}; #[inline(always)] /// Reads whitespace-separated tokens and passes each token to `f`. /// /// The input is processed a buffer at a time because `tsort` reads an /// unbounded stream of whitespace-separated tokens. -pub fn for_each_token(mut reader: R, mut f: F) -> io::Result<()> +pub fn for_each_token(mut reader: R, mut f: F) -> Result<(), Error> where R: BufRead, - F: FnMut(&[u8]), + F: FnMut(&[u8]) -> Result<(), Error>, { // Holds a partial token when the current buffer ends before its delimiter. let mut pending = Vec::new(); loop { - // Keep the borrow scoped for later call to consume. + // Keep the buffer borrow scoped so `consume` can be called afterwards. let consumed = { - let buf = reader.fill_buf()?; + let buf = reader.fill_buf().map_err(ReadError::Io)?; + if buf.is_empty() { // EOF => process any pending token. if !pending.is_empty() { - f(&pending); + f(&pending)?; } return Ok(()); } let mut start = 0; - // Find each delimiter in this Buf chunk. The bytes between `start` and + + // Find each delimiter in this buffer. The bytes between `start` and // a delimiter form one complete token. for end in memchr3_iter(b' ', b'\t', b'\n', buf) { if !pending.is_empty() { - // This token started in the previous chunk and ends here. - // try_reserve first to report the allocation failure. - pending.try_reserve(end - start)?; + // This token started in a previous buffer and ends here. + pending + .try_reserve(end - start) + .map_err(AllocationError::from)?; pending.extend_from_slice(&buf[start..end]); - f(&pending); + f(&pending)?; pending.clear(); } else if start != end { - // The token is entirely in this chunk, so avoid copying it. - f(&buf[start..end]); - } // else we encountered several whitespaces, keep going + // The token is entirely in this buffer, so avoid copying it. + f(&buf[start..end])?; + } + // Otherwise, consecutive whitespace: there is no token here. - // move past the separator to the next token. + // Move past the separator to the next token. start = end + 1; } if start != buf.len() { - // last token of the input (has no space after it). - pending.try_reserve(buf.len() - start)?; + // The final token in this buffer has no delimiter yet, so carry + // it into the next `fill_buf()` chunk. + pending + .try_reserve(buf.len() - start) + .map_err(AllocationError::from)?; pending.extend_from_slice(&buf[start..]); } - // end borrow for consumption. buf.len() }; diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index bfb63705af..6f3d3fa4dd 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -3,32 +3,37 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore TAOCP indegree // spell-checker:ignore (libs) interner mod error; +mod graph; mod interner; mod parser; use clap::{Arg, ArgAction, Command}; -use rustc_hash::FxHashMap; -use std::collections::VecDeque; -use std::collections::hash_map::Entry; use std::ffi::OsString; -use std::fmt; use std::fs::File; -use std::io::{self, BufRead, BufReader, BufWriter, Write}; +use std::io::{self, BufRead, BufReader}; use uucore::display::Quotable; use uucore::error::{FromIo, UError, UResult, USimpleError}; -use uucore::{format_usage, show, translate}; +use uucore::{format_usage, translate}; -use crate::error::{Error, ReadError}; -use crate::interner::{ByteInterner, Sym}; +use crate::error::{AllocationError, Error, ReadError}; +use crate::graph::GraphBuilder; +use crate::interner::Sym; mod options { pub const FILE: &str = "file"; } +#[inline] +fn try_clone_str(value: &str) -> Result { + let mut result = String::new(); + result.try_reserve(value.len())?; + result.push_str(value); + Ok(result) +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; @@ -51,7 +56,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } // Create the directed graph from pairs of tokens in the input data. - let mut g = Graph::new(input.to_string_lossy().to_string()); + let input_name = input.to_string_lossy(); + let graph_name = try_clone_str(&input_name).map_err(Error::from)?; + let mut g = GraphBuilder::new(graph_name); if input == "-" { process_input(io::stdin().lock(), &mut g)?; @@ -61,9 +68,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { { let input = std::path::Path::new(input); if input.is_dir() { - return Err( - Error::Read(ReadError::IsDir(input.to_string_lossy().to_string())).into(), - ); + let name = try_clone_str(&input.to_string_lossy()).map_err(Error::from)?; + return Err(Error::Read(ReadError::IsDir(name)).into()); } } @@ -77,6 +83,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { process_input(reader, &mut g)?; } + let mut g = g.finish(); g.run_tsort()?; Ok(()) } @@ -106,25 +113,9 @@ pub fn uu_app() -> Command { ) } -// Auxiliary struct, just for printing loop nodes via show! macro. -// -// Diagnostics go through Display, so invalid UTF-8 bytes are represented -// lossily here. -#[derive(Debug)] -struct LoopNode<'a>(&'a [u8]); - -impl fmt::Display for LoopNode<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", String::from_utf8_lossy(self.0)) - } -} - -impl std::error::Error for LoopNode<'_> {} - impl UError for Error {} -impl UError for LoopNode<'_> {} -fn process_input(reader: R, graph: &mut Graph) -> Result<(), Error> { +fn process_input(reader: R, graph: &mut GraphBuilder) -> Result<(), Error> { let mut pending: Option = None; // Input is considered to be in the format @@ -134,281 +125,29 @@ fn process_input(reader: R, graph: &mut Graph) -> Result<(), Error> // Tokens are kept as raw bytes so invalid UTF-8 can be preserved. let result = parser::for_each_token(reader, |token| { - let token_sym = graph.interner.get_or_intern(token); + let token_sym = graph.intern(token)?; if let Some(from) = pending.take() { - graph.add_edge(from, token_sym); + graph.add_edge(from, token_sym)?; } else { pending = Some(token_sym); } - }); - - if let Err(e) = result { - if e.kind() == io::ErrorKind::IsADirectory { - return Err(ReadError::IsDir(graph.name()).into()); - } - return Err(ReadError::Io(e).into()); - } - - if pending.is_some() { - return Err(ReadError::NumTokensOdd(graph.name()).into()); - } - graph.interner.finish_interning(); - Ok(()) -} - -/// Find the element `x` in `vec` and remove it, returning its index. -fn remove(vec: &mut Vec, x: T) -> Option -where - T: PartialEq, -{ - vec.iter().position(|item| *item == x).inspect(|i| { - vec.remove(*i); - }) -} - -#[derive(Clone, Copy, PartialEq, Eq)] -enum VisitedState { - Opened, - Closed, -} - -#[derive(Default)] -struct Node { - successor_tokens: Vec, - predecessor_count: usize, -} - -impl Node { - fn add_successor(&mut self, successor_name: Sym) { - self.successor_tokens.push(successor_name); - } -} - -struct Graph { - name: String, - nodes: FxHashMap, - interner: ByteInterner, -} - -impl Graph { - fn new(name: String) -> Self { - Self { - name, - nodes: FxHashMap::default(), - interner: ByteInterner::default(), - } - } - - fn name(&self) -> String { - self.name.clone() - } - - fn get_node_name(&self, node_sym: Sym) -> &[u8] { - self.interner - .resolve(node_sym) - .expect("symbol should be interned") - } - - fn add_edge(&mut self, from: Sym, to: Sym) { - let from_node = self.nodes.entry(from).or_default(); - - if from != to { - from_node.add_successor(to); - - let to_node = self.nodes.entry(to).or_default(); - to_node.predecessor_count += 1; - } - } - - fn remove_edge(&mut self, u: Sym, v: Sym) { - remove( - &mut self - .nodes - .get_mut(&u) - .expect("node is part of the graph") - .successor_tokens, - v, - ); - - self.nodes - .get_mut(&v) - .expect("node is part of the graph") - .predecessor_count -= 1; - } - - /// Implementation of algorithm T from TAOCP (Don. Knuth), vol. 1. - fn run_tsort(&mut self) -> Result<(), Error> { - let mut independent_nodes_queue: VecDeque = self - .nodes - .iter() - .filter_map(|(&sym, node)| { - if node.predecessor_count == 0 { - Some(sym) - } else { - None - } - }) - .collect(); - - // Sort by name for deterministic output. - independent_nodes_queue - .make_contiguous() - .sort_unstable_by(|a, b| self.get_node_name(*a).cmp(self.get_node_name(*b))); - let mut out = BufWriter::new(io::stdout().lock()); - - while !self.nodes.is_empty() { - let v = self.find_next_node(&mut independent_nodes_queue); - - // Write the node exactly as it appeared in the input, followed by a newline - out.write_all(self.get_node_name(v)).map_err(Error::Write)?; - writeln!(out).map_err(Error::Write)?; - - if let Some(node_to_process) = self.nodes.remove(&v) { - for successor_name in node_to_process.successor_tokens.into_iter().rev() { - // we reverse to match GNU tsort order - let successor_node = self - .nodes - .get_mut(&successor_name) - .expect("node is part of the graph"); - - successor_node.predecessor_count -= 1; - - if successor_node.predecessor_count == 0 { - independent_nodes_queue.push_back(successor_name); - } - } - } - } - - out.flush().map_err(Error::Write)?; Ok(()) - } - - pub fn indegree(&self, sym: Sym) -> Option { - self.nodes.get(&sym).map(|data| data.predecessor_count) - } - - fn find_next_node(&mut self, frontier: &mut VecDeque) -> Sym { - // If there are no nodes of in-degree zero but there are still - // un-visited nodes in the graph, then there must be a cycle. - // We need to find the cycle, display it on stderr, and break it to go on. - // - // A cycle is guaranteed to be of length at least two. We break - // the cycle by deleting an arbitrary edge (the first). That is - // not necessarily the optimal thing, but it should be enough to - // continue making progress in the graph traversal, and matches GNU tsort behavior. - // - // It is possible that deleting the edge does not actually - // result in the target node having in-degree zero, so we repeat - // the process until such a node appears. + }); - loop { - match frontier.pop_front() { - None => self.find_and_break_cycle(frontier), - Some(v) => return v, + if let Err(error) = result { + match error { + Error::Read(ReadError::Io(e)) if e.kind() == io::ErrorKind::IsADirectory => { + return Err(ReadError::IsDir(try_clone_str(graph.name())?).into()); } + error => return Err(error), } } - fn find_and_break_cycle(&mut self, frontier: &mut VecDeque) { - let cycle = self.detect_cycle(); - - show!(Error::Loop(self.name())); - - for &sym in &cycle { - show!(LoopNode(self.get_node_name(sym))); - } - - let u = *cycle.last().expect("cycle must be non-empty"); - let v = cycle[0]; - - self.remove_edge(u, v); - - if self.indegree(v).expect("node is part of the graph") == 0 { - frontier.push_back(v); - } - } - - fn detect_cycle(&self) -> Vec { - // Sort by name for deterministic output. - let mut nodes: Vec<_> = self.nodes.keys().copied().collect(); - - nodes.sort_unstable_by(|a, b| self.get_node_name(*a).cmp(self.get_node_name(*b))); - - let mut visited = FxHashMap::default(); - let mut stack = Vec::with_capacity(self.nodes.len()); - - for &node in &nodes { - if self.dfs(node, &mut visited, &mut stack) { - let (loop_entry, _) = stack.pop().expect("loop is not empty"); - - return stack - .into_iter() - .map(|(node, _)| node) - .skip_while(|&node| node != loop_entry) - .collect(); - } - } - - unreachable!("detect_cycle is expected to be called only on graphs with cycles"); + if pending.is_some() { + return Err(ReadError::NumTokensOdd(try_clone_str(graph.name())?).into()); } - fn dfs<'a>( - &'a self, - node: Sym, - visited: &mut FxHashMap, - stack: &mut Vec<(Sym, &'a [Sym])>, - ) -> bool { - stack.push(( - node, - self.nodes - .get(&node) - .map_or(&[], |n: &Node| &n.successor_tokens), - )); - - let state = *visited.entry(node).or_insert(VisitedState::Opened); - - if state == VisitedState::Closed { - return false; - } - - while let Some((node, pending_successors)) = stack.pop() { - let Some((&next_node, pending)) = pending_successors.split_first() else { - // no more pending successors in the list -> close the node - visited.insert(node, VisitedState::Closed); - continue; - }; - - // schedule processing for the pending part of successors for this node - stack.push((node, pending)); - - match visited.entry(next_node) { - Entry::Vacant(v) => { - // first visit of the node - v.insert(VisitedState::Opened); - - stack.push(( - next_node, - self.nodes - .get(&next_node) - .map_or(&[], |n| &n.successor_tokens), - )); - } - - Entry::Occupied(o) => { - if *o.get() == VisitedState::Opened { - // We have found a node that was already visited by another - // iteration => loop completed. The stack may contain - // unrelated nodes. This allows narrowing the loop down. - stack.push((next_node, &[])); - return true; - } - } - } - } - - false - } + Ok(()) }