From b1581f7aadc4761de560cf96f62d29759804f9fa Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Tue, 30 Sep 2025 12:35:24 -0700
Subject: [PATCH 01/15] Notes and cleanup and whatnot

---
 c2rust-transpile/src/cfg/loops.rs    |  4 +--
 c2rust-transpile/src/cfg/relooper.rs | 54 ++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/c2rust-transpile/src/cfg/loops.rs b/c2rust-transpile/src/cfg/loops.rs
index 05fb151f48..02e88f958a 100644
--- a/c2rust-transpile/src/cfg/loops.rs
+++ b/c2rust-transpile/src/cfg/loops.rs
@@ -47,7 +47,7 @@ pub fn match_loop_body(
             let bb = if let Some(bb) = follow_blocks.swap_remove(&following) {
                 bb
             } else {
-                continue;
+                continue; // Why would we have a follow entry with no corresponding block?
             };
             something_happened = true;
 
@@ -191,7 +191,7 @@ impl<Lbl: Hash + Eq + Clone> LoopInfo<Lbl> {
                         };
                     }
 
-                    None => return None,
+                    None => return None, // When would this happen? If we have a loop ID, why wouldn't it correspond to a loop?
                 }
             }
         }
diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index fcbe00a23e..71461ac62e 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -126,19 +126,22 @@ impl RelooperState {
     }
 }
 
+/// A set of basic blocks, keyed by their label.
+type StructuredBlocks = IndexMap<Label, BasicBlock<StructureLabel<StmtOrDecl>, StmtOrDecl>>;
+
 impl RelooperState {
     /// Recursive helper for `reloop`.
     ///
     /// TODO: perhaps manually perform TCO?
     fn relooper(
         &mut self,
-        entries: IndexSet<Label>, // current entry points into the CFG
-        mut blocks: IndexMap<Label, BasicBlock<StructureLabel<StmtOrDecl>, StmtOrDecl>>, // the blocks in the sub-CFG considered
+        entries: IndexSet<Label>,     // current entry points into the CFG
+        mut blocks: StructuredBlocks, // the blocks in the sub-CFG considered
         result: &mut Vec<Structure<StmtOrDecl>>, // the generated structures are appended to this
         disable_heuristics: bool,
     ) {
-        // Find nodes outside the graph pointed to from nodes inside the graph. Note that `ExitTo`
-        // is not considered here - only `GoTo`.
+        /// Find nodes outside the graph pointed to from nodes inside the graph. Note that `ExitTo`
+        /// is not considered here - only `GoTo`.
         fn out_edges<T>(
             blocks: &IndexMap<Label, BasicBlock<StructureLabel<StmtOrDecl>, T>>,
         ) -> IndexSet<Label> {
@@ -149,7 +152,7 @@ impl RelooperState {
                 .collect()
         }
 
-        // Transforms `{1: {'a', 'b'}, 2: {'b'}}` into `{'a': {1}, 'b': {1,2}}`.
+        /// Transforms `{1: {'a', 'b'}, 2: {'b'}}` into `{'a': {1}, 'b': {1,2}}`.
         fn flip_edges(map: IndexMap<Label, IndexSet<Label>>) -> IndexMap<Label, IndexSet<Label>> {
             let mut flipped_map: IndexMap<Label, IndexSet<Label>> = IndexMap::new();
             for (lbl, vals) in map {
@@ -160,9 +163,7 @@ impl RelooperState {
             flipped_map
         }
 
-        type StructuredBlocks = IndexMap<Label, BasicBlock<StructureLabel<StmtOrDecl>, StmtOrDecl>>;
-
-        // Find all labels reachable via a `GoTo` from the current set of blocks
+        // Find all labels reachable via a `GoTo` from the current set of blocks.
         let reachable_labels: IndexSet<Label> =
             blocks.iter().flat_map(|(_, bb)| bb.successors()).collect();
 
@@ -183,10 +184,11 @@ impl RelooperState {
         // Simple blocks
         if none_branch_to.len() == 1 && some_branch_to.is_empty() {
             let entry = none_branch_to
-                .iter()
-                .next()
+                .first()
                 .expect("Should find exactly one entry");
 
+            // legaren: What does the else case here mean? When would we have an entry
+            // that isn't in the current set of blocks?
             if let Some(bb) = blocks.swap_remove(entry) {
                 let new_entries = bb.successors();
                 let BasicBlock {
@@ -197,6 +199,8 @@ impl RelooperState {
                     span,
                 } = bb;
 
+                // legaren: What is this live/defined business?
+
                 // Flag declarations for everything that is live going in but not already in scope.
                 //
                 // It is tempting to just place the declarations here, but it isn't that simple:
@@ -294,7 +298,7 @@ impl RelooperState {
 
         // This information is necessary for both the `Loop` and `Multiple` cases
         let (predecessor_map, strict_reachable_from) = {
-            let successor_map: IndexMap<Label, IndexSet<Label>> = blocks
+            let successor_map = blocks
                 .iter()
                 .map(|(lbl, bb)| (lbl.clone(), bb.successors()))
                 .collect();
@@ -338,6 +342,7 @@ impl RelooperState {
                     visited.swap_remove(join);
                     if visited.difference(content).next().is_some() {
                         recognized_c_multiple = false;
+                        break;
                     }
                 }
             }
@@ -345,9 +350,10 @@ impl RelooperState {
         recognized_c_multiple = recognized_c_multiple && !disable_heuristics;
 
         if none_branch_to.is_empty() && !recognized_c_multiple {
+            // The set of nodes that can reach one of our current entries.
             let new_returns: IndexSet<Label> = strict_reachable_from
                 .iter()
-                .filter(|&(lbl, _)| blocks.contains_key(lbl) && entries.contains(lbl))
+                .filter(|&(lbl, _)| blocks.contains_key(lbl) && entries.contains(lbl)) // Is the blocks check redundant? If it's an entry doesn't it also have to be in `blocks`? If not, when?
                 .flat_map(|(_, reachable)| reachable.iter())
                 .cloned()
                 .collect();
@@ -401,6 +407,7 @@ impl RelooperState {
             }
 
             // Rename some `GoTo`s in the loop body to `ExitTo`s
+            // Why? Why is `ExitTo` here different than `GoTo`?
             for (_, bb) in body_blocks.iter_mut() {
                 for lbl in bb.terminator.get_labels_mut() {
                     if let StructureLabel::GoTo(label) = lbl.clone() {
@@ -434,7 +441,10 @@ impl RelooperState {
                 .insert(entry.clone());
         }
 
-        // Blocks that are reached by only one label
+        // Blocks that are reached by only one entry(?) I think this will also
+        // include the entries, since they were just added to `reachable_from`.
+        // Keys will be entries, values will be the blocks that are only
+        // reachable from that entry.
         let singly_reached: IndexMap<Label, IndexSet<Label>> = flip_edges(
             reachable_from
                 .into_iter()
@@ -443,6 +453,8 @@ impl RelooperState {
                 .collect(),
         );
 
+        // `singly_reached` but with the value labels replaced by the
+        // corresponding basic blocks from `blocks`.
         let handled_entries: IndexMap<Label, StructuredBlocks> = singly_reached
             .into_iter()
             .map(|(lbl, within)| {
@@ -455,12 +467,14 @@ impl RelooperState {
             })
             .collect();
 
+        // Entries that don't have any blocks that are only reachable from themselves.
         let unhandled_entries: IndexSet<Label> = entries
             .iter()
             .filter(|&e| !handled_entries.contains_key(e))
             .cloned()
             .collect();
 
+        // Gather the set of all blocks that are handled by one entry.
         let mut handled_blocks: StructuredBlocks = IndexMap::new();
         for (_, map) in &handled_entries {
             for (k, v) in map {
@@ -469,13 +483,19 @@ impl RelooperState {
         }
         let handled_blocks = handled_blocks;
 
+        // Gather the set of blocks that are not handled by any entry (or maybe
+        // are handled by multiple entries? unclear when that would come up or
+        // how that'd be handled).
         let follow_blocks: StructuredBlocks = blocks
             .into_iter()
             .filter(|(lbl, _)| !handled_blocks.contains_key(lbl))
             .collect();
 
+        // Unhandled entries (?) and any successors of handled blocks that are
+        // not themselves handled blocks.
         let follow_entries: IndexSet<Label> = &unhandled_entries | &out_edges(&handled_blocks);
 
+        // Reloop each of the handled blocks into their own structured control flow.
         let mut all_handlers: IndexMap<Label, Vec<Structure<StmtOrDecl>>> = handled_entries
             .into_iter()
             .map(|(lbl, blocks)| {
@@ -490,6 +510,12 @@ impl RelooperState {
             })
             .collect();
 
+        // If the set of handlers matches the set of entries, pull out the first
+        // handler (specifically the first handler added to `all_handles`, since
+        // we're using `IndexMap`) to use as the `then` block in the `Multiple`.
+        // Otherwise, our `then` block is empty.
+        //
+        // When would we fall into either case?
         let handler_keys: IndexSet<Label> = all_handlers.keys().cloned().collect();
         let (then, branches) = if handler_keys == entries {
             let a_key = all_handlers
@@ -503,7 +529,7 @@ impl RelooperState {
             (vec![], all_handlers)
         };
 
-        let disable_heuristics = follow_entries == entries;
+        let disable_heuristics = follow_entries == entries; // Why?
         result.push(Structure::Multiple {
             entries,
             branches,

From 01146e2243ff0028a2461b1057b023bd88b973a6 Mon Sep 17 00:00:00 2001
From: Nicole L <dlegare.1001@gmail.com>
Date: Tue, 30 Sep 2025 15:14:03 -0700
Subject: [PATCH 02/15] Document transitive_closure and friends

---
 c2rust-transpile/src/cfg/relooper.rs | 29 ++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 71461ac62e..d20c512e72 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -176,12 +176,14 @@ impl RelooperState {
 
         // --------------------------------------
         // Base case
+
         if none_branch_to.is_empty() && some_branch_to.is_empty() {
             return;
         }
 
         // --------------------------------------
         // Simple blocks
+
         if none_branch_to.len() == 1 && some_branch_to.is_empty() {
             let entry = none_branch_to
                 .first()
@@ -270,7 +272,13 @@ impl RelooperState {
         // --------------------------------------
         // Loops
 
-        // DFS transitive closure
+        /// Calculates the transitive closure of a directed graph via
+        /// depth-first search.
+        ///
+        /// Given an adjacency list, represented as a map from each label to its
+        /// immediate successors, calculate which labels are reachable from each
+        /// starting label. A label does not count as reachable from itself
+        /// unless there is a back edge from one of its successors.
         fn transitive_closure<V: Clone + Hash + Eq>(
             adjacency_list: &IndexMap<V, IndexSet<V>>,
         ) -> IndexMap<V, IndexSet<V>> {
@@ -296,14 +304,31 @@ impl RelooperState {
             closure
         }
 
-        // This information is necessary for both the `Loop` and `Multiple` cases
+        // Calculate predecessor and reachability information:
+        //
+        // * `predecessor_map` maps from each label to its immediate predecessors in the
+        //   current set of blocks.
+        // * `strict_reachable_from` maps each label to the set of labels it can reach
+        //   (not including itself).
+        //
+        // For both of these, some keys may not correspond to any blocks in the current
+        // set, since blocks may have successors that are outside the current set. For
+        // both of these, labels in the values set will always correspond to blocks in
+        // the current set.
         let (predecessor_map, strict_reachable_from) = {
+            // Map each of our current blocks to its immediate successors.
             let successor_map = blocks
                 .iter()
                 .map(|(lbl, bb)| (lbl.clone(), bb.successors()))
                 .collect();
 
+            // Calculate the transitive closure of the successor map, i.e. the full set of
+            // labels reachable from each block, not including itself. Then flip the edges
+            // to get a map of labels that can reach each block.
             let strict_reachable_from = flip_edges(transitive_closure(&successor_map));
+
+            // Flip the edges of the successor map to get a map of immediate predecessors
+            // for each block.
             let predecessor_map = flip_edges(successor_map);
 
             (predecessor_map, strict_reachable_from)

From c072a7e85deac1eee0dec6caed5b0e1b03052e1e Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Tue, 30 Sep 2025 17:26:17 -0700
Subject: [PATCH 03/15] Moar comments

---
 c2rust-transpile/src/cfg/relooper.rs | 73 ++++++++++++++++------------
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index d20c512e72..323aff81ac 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -1,5 +1,13 @@
-//! This modules handles converting a a control-flow graph `Cfg` into `Vec<Structure>`, optionally
-//! simplifying the latter.
+//! This modules handles converting a a control-flow graph `Cfg` into
+//! `Vec<Structure>`, optionally simplifying the latter.
+//!
+//! # Terminology
+//!
+//! The terms "label" and "block" are sometimes used interchangeably. A label is
+//! the unique identifier for a block. In some cases we're working directly with
+//! the basic blocks, but in many places when working with the control-flow
+//! graph we're dealing only with labels, and the blocks themselves are
+//! secondary.
 
 use super::*;
 
@@ -272,13 +280,13 @@ impl RelooperState {
         // --------------------------------------
         // Loops
 
-        /// Calculates the transitive closure of a directed graph via
-        /// depth-first search.
+        /// Calculates the transitive closure of a directed graph via depth-first
+        /// search.
         ///
         /// Given an adjacency list, represented as a map from each label to its
         /// immediate successors, calculate which labels are reachable from each
-        /// starting label. A label does not count as reachable from itself
-        /// unless there is a back edge from one of its successors.
+        /// starting label. A label does not count as reachable from itself unless there
+        /// is a back edge from one of its successors.
         fn transitive_closure<V: Clone + Hash + Eq>(
             adjacency_list: &IndexMap<V, IndexSet<V>>,
         ) -> IndexMap<V, IndexSet<V>> {
@@ -308,8 +316,10 @@ impl RelooperState {
         //
         // * `predecessor_map` maps from each label to its immediate predecessors in the
         //   current set of blocks.
-        // * `strict_reachable_from` maps each label to the set of labels it can reach
-        //   (not including itself).
+        // * `strict_reachable_from` maps each label to the set of labels that can reach
+        //   it, i.e. its direct and indirect predecessors. Entries do not count as
+        //   reaching themselves, and so will only appear as keys if they are reachable
+        //   from some other block.
         //
         // For both of these, some keys may not correspond to any blocks in the current
         // set, since blocks may have successors that are outside the current set. For
@@ -324,7 +334,7 @@ impl RelooperState {
 
             // Calculate the transitive closure of the successor map, i.e. the full set of
             // labels reachable from each block, not including itself. Then flip the edges
-            // to get a map of labels that can reach each block.
+            // to get a map from a label to the set of labels that can reach it.
             let strict_reachable_from = flip_edges(transitive_closure(&successor_map));
 
             // Flip the edges of the successor map to get a map of immediate predecessors
@@ -457,7 +467,7 @@ impl RelooperState {
         // --------------------------------------
         // Multiple
 
-        // Like `strict_reachable_from`, but entries also reach themselves
+        // Like `strict_reachable_from`, but entries also reach themselves.
         let mut reachable_from: IndexMap<Label, IndexSet<Label>> = strict_reachable_from;
         for entry in &entries {
             reachable_from
@@ -466,20 +476,24 @@ impl RelooperState {
                 .insert(entry.clone());
         }
 
-        // Blocks that are reached by only one entry(?) I think this will also
-        // include the entries, since they were just added to `reachable_from`.
-        // Keys will be entries, values will be the blocks that are only
-        // reachable from that entry.
+        // Calculate which blocks are reached by only one entry (including the entries
+        // themselves).
+        //
+        // For each label in `reachable_from`, filter its predecessor set to only our
+        // entries, then filter down to only the labels reachable from exactly one
+        // entry. Then flip edges to give us a map from entries to the set of labels
+        // that are only reached from that entry.
         let singly_reached: IndexMap<Label, IndexSet<Label>> = flip_edges(
             reachable_from
                 .into_iter()
-                .map(|(lbl, reachable)| (lbl, &reachable & &entries.clone()))
+                .map(|(lbl, reachable)| (lbl, &reachable & &entries))
                 .filter(|(_, reachable)| reachable.len() == 1)
                 .collect(),
         );
 
-        // `singly_reached` but with the value labels replaced by the
-        // corresponding basic blocks from `blocks`.
+        // Map from entry labels to the set of blocks only reachable from that entry,
+        // i.e. `singly_reached` but with the set of reachable labels replaced by the
+        // corresponding blocks.
         let handled_entries: IndexMap<Label, StructuredBlocks> = singly_reached
             .into_iter()
             .map(|(lbl, within)| {
@@ -499,25 +513,24 @@ impl RelooperState {
             .cloned()
             .collect();
 
-        // Gather the set of all blocks that are handled by one entry.
-        let mut handled_blocks: StructuredBlocks = IndexMap::new();
-        for (_, map) in &handled_entries {
-            for (k, v) in map {
-                handled_blocks.entry(k.clone()).or_insert(v.clone());
-            }
-        }
-        let handled_blocks = handled_blocks;
+        // Gather the set of all blocks that are only reachable from one entry
+        // (including the entries if they are only reachable from themselves).
+        let handled_blocks: StructuredBlocks = handled_entries
+            .values()
+            .flatten()
+            .map(|(k, v)| (k.clone(), v.clone()))
+            .collect();
 
-        // Gather the set of blocks that are not handled by any entry (or maybe
-        // are handled by multiple entries? unclear when that would come up or
-        // how that'd be handled).
+        // Gather the unhandled blocks, i.e. any blocks that are reachable from more
+        // than one entry. These become our "follow" blocks, i.e. the blocks that come
+        // after the `Multiple`.
         let follow_blocks: StructuredBlocks = blocks
             .into_iter()
             .filter(|(lbl, _)| !handled_blocks.contains_key(lbl))
             .collect();
 
-        // Unhandled entries (?) and any successors of handled blocks that are
-        // not themselves handled blocks.
+        // The entries for the follow blocks are our unhandled entries and any successors
+        // of handled blocks that are not themselves handled blocks.
         let follow_entries: IndexSet<Label> = &unhandled_entries | &out_edges(&handled_blocks);
 
         // Reloop each of the handled blocks into their own structured control flow.

From 838d8f22cfcbd02e85abb3640b61b57d1ec74520 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Wed, 1 Oct 2025 11:29:29 -0700
Subject: [PATCH 04/15] More judgemental comments

---
 c2rust-transpile/src/cfg/relooper.rs | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 323aff81ac..debc356d2b 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -534,12 +534,12 @@ impl RelooperState {
         let follow_entries: IndexSet<Label> = &unhandled_entries | &out_edges(&handled_blocks);
 
         // Reloop each of the handled blocks into their own structured control flow.
-        let mut all_handlers: IndexMap<Label, Vec<Structure<StmtOrDecl>>> = handled_entries
+        let mut all_handlers: IndexMap<_, _> = handled_entries
             .into_iter()
             .map(|(lbl, blocks)| {
                 let entries = indexset![lbl.clone()];
 
-                let mut structs: Vec<Structure<StmtOrDecl>> = vec![];
+                let mut structs = vec![];
                 self.open_scope();
                 self.relooper(entries, blocks, &mut structs, false);
                 self.close_scope();
@@ -548,14 +548,16 @@ impl RelooperState {
             })
             .collect();
 
-        // If the set of handlers matches the set of entries, pull out the first
-        // handler (specifically the first handler added to `all_handles`, since
-        // we're using `IndexMap`) to use as the `then` block in the `Multiple`.
-        // Otherwise, our `then` block is empty.
+        // If all entries are handled, we grab the first one to be the "then"
+        // branch, otherwise the "then" branch is empty.
         //
-        // When would we fall into either case?
+        // legaren: Why? What's the "then" branch for? The original relooper
+        // paper doesn't have an equivalent for this.
         let handler_keys: IndexSet<Label> = all_handlers.keys().cloned().collect();
         let (then, branches) = if handler_keys == entries {
+            // TODO: We can probably just `swap_remove_index(0)` here instead of
+            // getting the first key, I think that should be equivalent to what
+            // we're doing.
             let a_key = all_handlers
                 .keys()
                 .next()

From 222ae4178f3e6b7170ea1d333acfc21063e64a17 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Wed, 1 Oct 2025 12:37:49 -0700
Subject: [PATCH 05/15] More comments and such

---
 c2rust-transpile/src/cfg/relooper.rs | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index debc356d2b..a09a485bba 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -3,11 +3,11 @@
 //!
 //! # Terminology
 //!
-//! The terms "label" and "block" are sometimes used interchangeably. A label is
-//! the unique identifier for a block. In some cases we're working directly with
-//! the basic blocks, but in many places when working with the control-flow
-//! graph we're dealing only with labels, and the blocks themselves are
-//! secondary.
+//! The terms "label", "block", and "node" are sometimes used interchangeably.
+//! A label is the unique identifier for a block, and a block is a node in the
+//! control-flow graph. In some cases we're working directly with the basic
+//! blocks, but in many places when working with the control-flow graph we're
+//! dealing only with labels, and the blocks themselves are secondary.
 
 use super::*;
 
@@ -318,8 +318,8 @@ impl RelooperState {
         //   current set of blocks.
         // * `strict_reachable_from` maps each label to the set of labels that can reach
         //   it, i.e. its direct and indirect predecessors. Entries do not count as
-        //   reaching themselves, and so will only appear as keys if they are reachable
-        //   from some other block.
+        //   reaching themselves (unless they explicitly branch to themselves), and so
+        //   will only appear as keys if they are the predecessor of some block.
         //
         // For both of these, some keys may not correspond to any blocks in the current
         // set, since blocks may have successors that are outside the current set. For
@@ -385,10 +385,10 @@ impl RelooperState {
         recognized_c_multiple = recognized_c_multiple && !disable_heuristics;
 
         if none_branch_to.is_empty() && !recognized_c_multiple {
-            // The set of nodes that can reach one of our current entries.
+            // Gather the set of current blocks that can reach one of our entries.
             let new_returns: IndexSet<Label> = strict_reachable_from
                 .iter()
-                .filter(|&(lbl, _)| blocks.contains_key(lbl) && entries.contains(lbl)) // Is the blocks check redundant? If it's an entry doesn't it also have to be in `blocks`? If not, when?
+                .filter(|&(lbl, _)| blocks.contains_key(lbl) && entries.contains(lbl))
                 .flat_map(|(_, reachable)| reachable.iter())
                 .cloned()
                 .collect();
@@ -397,9 +397,14 @@ impl RelooperState {
             let (mut body_blocks, mut follow_blocks): (StructuredBlocks, StructuredBlocks) = blocks
                 .into_iter()
                 .partition(|(lbl, _)| new_returns.contains(lbl) || entries.contains(lbl));
+
+            // Any block that follows a body block but is not itself a body block
+            // becomes an entry for the follow blocks.
             let mut follow_entries = out_edges(&body_blocks);
 
             // Try to match an existing loop (from the initial C)
+            //
+            // legaren: Figure out how this part works.
             let mut matched_existing_loop = false;
             if let Some(ref loop_info) = self.loop_info {
                 let must_be_in_loop = entries.iter().chain(new_returns.iter()).cloned();
@@ -443,7 +448,7 @@ impl RelooperState {
 
             // Rename some `GoTo`s in the loop body to `ExitTo`s
             // Why? Why is `ExitTo` here different than `GoTo`?
-            for (_, bb) in body_blocks.iter_mut() {
+            for bb in body_blocks.values_mut() {
                 for lbl in bb.terminator.get_labels_mut() {
                     if let StructureLabel::GoTo(label) = lbl.clone() {
                         if entries.contains(&label) || follow_entries.contains(&label) {
@@ -468,7 +473,7 @@ impl RelooperState {
         // Multiple
 
         // Like `strict_reachable_from`, but entries also reach themselves.
-        let mut reachable_from: IndexMap<Label, IndexSet<Label>> = strict_reachable_from;
+        let mut reachable_from = strict_reachable_from;
         for entry in &entries {
             reachable_from
                 .entry(entry.clone())

From 35440c4460233d97b7328895e53b49410790bb29 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Wed, 1 Oct 2025 14:02:47 -0700
Subject: [PATCH 06/15] Document why we rewrite GoTo to ExitTo in loops

---
 c2rust-transpile/src/cfg/relooper.rs | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index a09a485bba..cba35c2e9c 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -286,7 +286,8 @@ impl RelooperState {
         /// Given an adjacency list, represented as a map from each label to its
         /// immediate successors, calculate which labels are reachable from each
         /// starting label. A label does not count as reachable from itself unless there
-        /// is a back edge from one of its successors.
+        /// is a back edge from one of its successors (including if the block is a
+        /// successor of itself).
         fn transitive_closure<V: Clone + Hash + Eq>(
             adjacency_list: &IndexMap<V, IndexSet<V>>,
         ) -> IndexMap<V, IndexSet<V>> {
@@ -384,6 +385,10 @@ impl RelooperState {
         }
         recognized_c_multiple = recognized_c_multiple && !disable_heuristics;
 
+        // If we have entries that are branched to (which is true at this point) and
+        // there are no entries outside the loop (i.e. all of our entries are branched
+        // back to at some point), and we didn't find an existing C multiple that
+        // applies, produce a loop.
         if none_branch_to.is_empty() && !recognized_c_multiple {
             // Gather the set of current blocks that can reach one of our entries.
             let new_returns: IndexSet<Label> = strict_reachable_from
@@ -446,8 +451,19 @@ impl RelooperState {
                 );
             }
 
-            // Rename some `GoTo`s in the loop body to `ExitTo`s
-            // Why? Why is `ExitTo` here different than `GoTo`?
+            // Rewrite `GoTo`s that exit the loop body to `ExitTo`s.
+            //
+            // For our body blocks, rewrite terminator `GoTo` labels to `ExitTo` if they
+            // target an entry or a follow entry. These are exits from the loop body, i.e.
+            // if they branch back to an entry then it's a `continue`, and if they branch to
+            // a follow entry then it's a `break`.
+            //
+            // This is necessary so that when we reloop the body blocks it doesn't get
+            // tripped up by the back/out edges. Without this, the reloop pass for the loop
+            // body would see the back edges and decide to produce a loop structure, even
+            // though we've already done that. Changing from `GoTo` to `ExitTo` means that
+            // the back/out edges don't show up in the list of successors when calculating
+            // predecessor and reachability information.
             for bb in body_blocks.values_mut() {
                 for lbl in bb.terminator.get_labels_mut() {
                     if let StructureLabel::GoTo(label) = lbl.clone() {

From e0fa89f5a9aacd309997bda480caea6e890014f4 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Wed, 1 Oct 2025 16:26:38 -0700
Subject: [PATCH 07/15] Document the logic for matching a C multiple

---
 c2rust-transpile/src/cfg/relooper.rs | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index cba35c2e9c..61fcfe79b3 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -260,6 +260,7 @@ impl RelooperState {
             .cloned()
             .partition(|entry| blocks.contains_key(entry));
 
+        // legaren: When does this happen and what does it do?
         if !absent.is_empty() {
             if !present.is_empty() {
                 let branches = absent.into_iter().map(|lbl| (lbl, vec![])).collect();
@@ -353,10 +354,23 @@ impl RelooperState {
             if let Some((join, arms)) = multiple_info.get_multiple(&entries_key) {
                 recognized_c_multiple = true;
 
-                for (entry, content) in arms {
+                // Search through the arms of the C multiple to verify that our current CFG matches
+                // the structure of the C multiple.
+                'search: for (entry, content) in arms {
                     let mut to_visit: Vec<Label> = vec![entry.clone()];
                     let mut visited: IndexSet<Label> = IndexSet::new();
 
+                    // Walk the graph forwards from the entry, making sure we only see blocks that
+                    // are part of the current multiple. If we find any nodes that aren't in
+                    // `content` then the CFG differs from the C multiple.
+                    //
+                    // NOTE: We can't reuse the previous reachability analysis here because it
+                    // includes nodes past the join node, which isn't what we want here.
+                    //
+                    // legaren: Actually I'm not 100% sure about that last part, if we restructure
+                    // the logic here a bit we may be able to reuse `strict_reachable_from` or
+                    // `transitive_closure`, but I haven't thought through it enough yet and I'd
+                    // want to have tests before playing with this logic anyway.
                     while let Some(lbl) = to_visit.pop() {
                         // Stop at things you've already seen or the join block
                         if !visited.insert(lbl.clone()) || lbl == *join {
@@ -364,21 +378,22 @@ impl RelooperState {
                         }
 
                         if let Some(bb) = blocks.get(&lbl) {
-                            // If this isn't something we are supposed to encounter, break and fail.
                             if !content.contains(&lbl) {
                                 recognized_c_multiple = false;
-                                break;
+                                break 'search;
                             }
 
                             to_visit.extend(bb.successors())
                         }
                     }
 
-                    // Check we've actually visited all of the expected content
+                    // Check that we've only visited the expected nodes. If we've visited any nodes
+                    // not in `content`, then the CFG differs from the C multiple and the search
+                    // fails.
                     visited.swap_remove(join);
                     if visited.difference(content).next().is_some() {
                         recognized_c_multiple = false;
-                        break;
+                        break 'search;
                     }
                 }
             }

From 4aa926adffb10a9d6956d7847cd2c5ec58a67081 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Thu, 2 Oct 2025 18:04:26 -0700
Subject: [PATCH 08/15] Document how/why absent entries are handled

---
 c2rust-transpile/src/cfg/relooper.rs | 32 ++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 61fcfe79b3..7243a096f8 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -260,7 +260,18 @@ impl RelooperState {
             .cloned()
             .partition(|entry| blocks.contains_key(entry));
 
-        // legaren: When does this happen and what does it do?
+        // Handle the case where we have entries that are not in the current set of
+        // blocks.
+        //
+        // This happens when we create a multiple where a block reachable from multiple
+        // entries gets put in the follow blocks. This means that the branches within
+        // the multiple that reach the follow block have to do nothing, since they need
+        // to exit the multiple to get to that next block. To handle this we create a
+        // multiple with no-op branches that correspond to the absent entries, and a
+        // then branch with result of relooping the present entries.
+        //
+        // If we only have absent entries, then there's no other blocks to structure and
+        // we can just exit.
         if !absent.is_empty() {
             if !present.is_empty() {
                 let branches = absent.into_iter().map(|lbl| (lbl, vec![])).collect();
@@ -272,8 +283,8 @@ impl RelooperState {
                     entries,
                     branches,
                     then,
-                })
-            };
+                });
+            }
 
             return;
         }
@@ -355,7 +366,7 @@ impl RelooperState {
                 recognized_c_multiple = true;
 
                 // Search through the arms of the C multiple to verify that our current CFG matches
-                // the structure of the C multiple.
+                // its structure.
                 'search: for (entry, content) in arms {
                     let mut to_visit: Vec<Label> = vec![entry.clone()];
                     let mut visited: IndexSet<Label> = IndexSet::new();
@@ -404,6 +415,15 @@ impl RelooperState {
         // there are no entries outside the loop (i.e. all of our entries are branched
         // back to at some point), and we didn't find an existing C multiple that
         // applies, produce a loop.
+        //
+        // legaren: This loop analysis is a bit simple, in that if `none_branch_to` is
+        // empty it assumes there's only one loop. If we have multiple disjoint loops
+        // it'll still only generate a single loop structure, which then has a multiple
+        // inside of it that handles both loop bodies as an awkward state machine. It
+        // may be better to instead generate the multiple first, which will then give us
+        // separate loops. I'm not 100% sure that'd be better in all cases, and the
+        // above logic for trying to reconstruct C multiples may already solve this
+        // issue in practice.
         if none_branch_to.is_empty() && !recognized_c_multiple {
             // Gather the set of current blocks that can reach one of our entries.
             let new_returns: IndexSet<Label> = strict_reachable_from
@@ -569,7 +589,7 @@ impl RelooperState {
         // of handled blocks that are not themselves handled blocks.
         let follow_entries: IndexSet<Label> = &unhandled_entries | &out_edges(&handled_blocks);
 
-        // Reloop each of the handled blocks into their own structured control flow.
+        // Reloop each set of handled blocks into their own structured control flow.
         let mut all_handlers: IndexMap<_, _> = handled_entries
             .into_iter()
             .map(|(lbl, blocks)| {
@@ -605,7 +625,7 @@ impl RelooperState {
             (vec![], all_handlers)
         };
 
-        let disable_heuristics = follow_entries == entries; // Why?
+        let disable_heuristics = follow_entries == entries; // legaren: Why?
         result.push(Structure::Multiple {
             entries,
             branches,

From 2192be102130b60371a5beb394a52c92f9210bd4 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Fri, 3 Oct 2025 11:08:48 -0700
Subject: [PATCH 09/15] Cleanup some comments

---
 c2rust-transpile/src/cfg/loops.rs    |  4 ++--
 c2rust-transpile/src/cfg/relooper.rs | 10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/c2rust-transpile/src/cfg/loops.rs b/c2rust-transpile/src/cfg/loops.rs
index 02e88f958a..6141c36c06 100644
--- a/c2rust-transpile/src/cfg/loops.rs
+++ b/c2rust-transpile/src/cfg/loops.rs
@@ -47,7 +47,7 @@ pub fn match_loop_body(
             let bb = if let Some(bb) = follow_blocks.swap_remove(&following) {
                 bb
             } else {
-                continue; // Why would we have a follow entry with no corresponding block?
+                continue; // legaren: Why would we have a follow entry with no corresponding block?
             };
             something_happened = true;
 
@@ -191,7 +191,7 @@ impl<Lbl: Hash + Eq + Clone> LoopInfo<Lbl> {
                         };
                     }
 
-                    None => return None, // When would this happen? If we have a loop ID, why wouldn't it correspond to a loop?
+                    None => return None, // legaren: When would this happen? If we have a loop ID, why wouldn't it correspond to a loop?
                 }
             }
         }
diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 7243a096f8..6d7568f64e 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -197,8 +197,12 @@ impl RelooperState {
                 .first()
                 .expect("Should find exactly one entry");
 
-            // legaren: What does the else case here mean? When would we have an entry
-            // that isn't in the current set of blocks?
+            // If our entry is in our current blocks, emit it as a simple block and then
+            // recurse into the rest of the blocks.
+            //
+            // If our entry is not in our current blocks, then it was previously selected as
+            // the follow blocks of a multiple and we are at the end of that branch. In this
+            // case we emit an empty simple block that jumps to the entry.
             if let Some(bb) = blocks.swap_remove(entry) {
                 let new_entries = bb.successors();
                 let BasicBlock {
@@ -209,8 +213,6 @@ impl RelooperState {
                     span,
                 } = bb;
 
-                // legaren: What is this live/defined business?
-
                 // Flag declarations for everything that is live going in but not already in scope.
                 //
                 // It is tempting to just place the declarations here, but it isn't that simple:

From d0e9b7c032d7287c44f994d6dca2d5212c5becc0 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Fri, 3 Oct 2025 15:38:51 -0700
Subject: [PATCH 10/15] Add more comments about loop stuff

---
 c2rust-transpile/src/cfg/loops.rs    | 31 ++++++++++++++--------------
 c2rust-transpile/src/cfg/relooper.rs | 14 ++++++++-----
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/c2rust-transpile/src/cfg/loops.rs b/c2rust-transpile/src/cfg/loops.rs
index 6141c36c06..d99777974a 100644
--- a/c2rust-transpile/src/cfg/loops.rs
+++ b/c2rust-transpile/src/cfg/loops.rs
@@ -170,28 +170,27 @@ impl<Lbl: Hash + Eq + Clone> LoopInfo<Lbl> {
         self.loops.extend(other.loops);
     }
 
-    /// Find the smallest possible loop that contains all of the items
+    /// Finds the smallest possible loop that contains all of the entries.
     pub fn tightest_common_loop<E: Iterator<Item = Lbl>>(&self, mut entries: E) -> Option<LoopId> {
+        // Start with the loop containing the first entry.
         let first = entries.next()?;
-
         let mut loop_id = *self.node_loops.get(&first)?;
 
+        // Widen the loop until it contains the all of our entries, or it can no longer
+        // be widened.
         for entry in entries {
-            // Widen the loop until it contains the `entry`, or it can no longer be widened.
             loop {
-                match self.loops.get(&loop_id) {
-                    Some((ref in_loop, parent_id)) => {
-                        if in_loop.contains(&entry) {
-                            break;
-                        }
-                        loop_id = if let Some(i) = parent_id {
-                            *i
-                        } else {
-                            return None;
-                        };
-                    }
-
-                    None => return None, // legaren: When would this happen? If we have a loop ID, why wouldn't it correspond to a loop?
+                // legaren: Can we ever get `None` out of `get` here? That would mean we have a
+                // loop ID with no corresponding entry in `loops`, which seems invalid. If this
+                // isn't a valid case we should unwrap/expect here instead of using `?`.
+                let (in_loop, parent_id) = self.loops.get(&loop_id)?;
+
+                // If our current loop contains the entry, move on to the next entry. Otherwise
+                // move on to the next wider loop if there is one.
+                if in_loop.contains(&entry) {
+                    break;
+                } else {
+                    loop_id = parent_id.clone()?;
                 }
             }
         }
diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 6d7568f64e..918ee94353 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -333,8 +333,9 @@ impl RelooperState {
         //   current set of blocks.
         // * `strict_reachable_from` maps each label to the set of labels that can reach
         //   it, i.e. its direct and indirect predecessors. Entries do not count as
-        //   reaching themselves (unless they explicitly branch to themselves), and so
-        //   will only appear as keys if they are the predecessor of some block.
+        //   reaching themselves, and so an entry will only appear as a key if it is the
+        //   successor of some block (including itself, if the entry branches directly
+        //   to itself).
         //
         // For both of these, some keys may not correspond to any blocks in the current
         // set, since blocks may have successors that are outside the current set. For
@@ -427,7 +428,12 @@ impl RelooperState {
         // above logic for trying to reconstruct C multiples may already solve this
         // issue in practice.
         if none_branch_to.is_empty() && !recognized_c_multiple {
-            // Gather the set of current blocks that can reach one of our entries.
+            // Gather the set of current blocks that can reach one of our entries, not
+            // including the entries themselves unless they are the successor of some
+            // block (including itself, if the entry explicitly branches to itself).
+            //
+            // NOTE: At least one entry will be present here since at this point we know
+            // that there is an entry that is branched to.
             let new_returns: IndexSet<Label> = strict_reachable_from
                 .iter()
                 .filter(|&(lbl, _)| blocks.contains_key(lbl) && entries.contains(lbl))
@@ -445,8 +451,6 @@ impl RelooperState {
             let mut follow_entries = out_edges(&body_blocks);
 
             // Try to match an existing loop (from the initial C)
-            //
-            // legaren: Figure out how this part works.
             let mut matched_existing_loop = false;
             if let Some(ref loop_info) = self.loop_info {
                 let must_be_in_loop = entries.iter().chain(new_returns.iter()).cloned();

From 5d29b507056ba70bf6238e87c286e629dfdbb32d Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Mon, 6 Oct 2025 13:47:40 -0700
Subject: [PATCH 11/15] Document more of the subtleties of multiple creation

---
 c2rust-transpile/src/cfg/loops.rs    |  2 +-
 c2rust-transpile/src/cfg/relooper.rs | 46 ++++++++++++++++++----------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/c2rust-transpile/src/cfg/loops.rs b/c2rust-transpile/src/cfg/loops.rs
index d99777974a..363c12d5a5 100644
--- a/c2rust-transpile/src/cfg/loops.rs
+++ b/c2rust-transpile/src/cfg/loops.rs
@@ -47,7 +47,7 @@ pub fn match_loop_body(
             let bb = if let Some(bb) = follow_blocks.swap_remove(&following) {
                 bb
             } else {
-                continue; // legaren: Why would we have a follow entry with no corresponding block?
+                continue;
             };
             something_happened = true;
 
diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 918ee94353..730bdcfcf1 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -1,5 +1,5 @@
-//! This modules handles converting a a control-flow graph `Cfg` into
-//! `Vec<Structure>`, optionally simplifying the latter.
+//! This modules handles converting a a control-flow graph `Cfg` into `Vec<Structure>`, optionally
+//! simplifying the latter.
 //!
 //! # Terminology
 //!
@@ -610,33 +610,47 @@ impl RelooperState {
             })
             .collect();
 
-        // If all entries are handled, we grab the first one to be the "then"
-        // branch, otherwise the "then" branch is empty.
+        // If all entries are handled, we grab the first one to be the "then" branch,
+        // otherwise the "then" branch is empty.
         //
-        // legaren: Why? What's the "then" branch for? The original relooper
-        // paper doesn't have an equivalent for this.
+        // This is done because of how we generate `match` statements from `Multiple`.
+        // If all entries are handled, then we pull one case to be the `_ => { ... }`
+        // default case in the generated `match`. This works because every entry will
+        // have its own `match` arm, so it's safe to have the `_` arm cover one of them.
+        //
+        // If we have unhandled entries, then we'll have entries not covered by the
+        // `match`. In this case we want an empty `_ => {}` case, that way when we hit
+        // the `match` from one of the unhandled entries we just fall through the
+        // `match` and continue on to the correct entry.
         let handler_keys: IndexSet<Label> = all_handlers.keys().cloned().collect();
         let (then, branches) = if handler_keys == entries {
-            // TODO: We can probably just `swap_remove_index(0)` here instead of
-            // getting the first key, I think that should be equivalent to what
-            // we're doing.
-            let a_key = all_handlers
-                .keys()
-                .next()
-                .expect("no handlers found")
-                .clone();
-            let last_handler = all_handlers.swap_remove(&a_key).expect("just got this key");
+            let last_handler = all_handlers
+                .swap_remove_index(0)
+                .expect("There has to be at least one handler")
+                .1;
             (last_handler, all_handlers)
         } else {
             (vec![], all_handlers)
         };
 
-        let disable_heuristics = follow_entries == entries; // legaren: Why?
+        // Disable heuristics when relooping the follow blocks if all of our entries are
+        // follow entries.
+        //
+        // This situation happens when there are back edges to all of our entries, i.e.
+        // when all of our entries are part of one or more loops. If we leave heuristics
+        // enabled in this case, we risk infinite recursion because we're relooping the
+        // same set of blocks with the same set of entries. If there are loops and we
+        // ended up here, it means that we skipped creating a loop because we matched a
+        // C multiple. Disabling heuristics means we won't try to match a C multiple,
+        // meaning we will generate a loop-match instead.
+        let disable_heuristics = follow_entries == entries;
+
         result.push(Structure::Multiple {
             entries,
             branches,
             then,
         });
+
         self.relooper(follow_entries, follow_blocks, result, disable_heuristics);
     }
 }

From fd722cca45db3792ef7fc597558eae4b7a7aab44 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Mon, 6 Oct 2025 14:02:38 -0700
Subject: [PATCH 12/15] Assert on unrecognized loop IDs

---
 c2rust-transpile/src/cfg/loops.rs | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/c2rust-transpile/src/cfg/loops.rs b/c2rust-transpile/src/cfg/loops.rs
index 363c12d5a5..1ac69a4617 100644
--- a/c2rust-transpile/src/cfg/loops.rs
+++ b/c2rust-transpile/src/cfg/loops.rs
@@ -180,10 +180,9 @@ impl<Lbl: Hash + Eq + Clone> LoopInfo<Lbl> {
         // be widened.
         for entry in entries {
             loop {
-                // legaren: Can we ever get `None` out of `get` here? That would mean we have a
-                // loop ID with no corresponding entry in `loops`, which seems invalid. If this
-                // isn't a valid case we should unwrap/expect here instead of using `?`.
-                let (in_loop, parent_id) = self.loops.get(&loop_id)?;
+                let (in_loop, parent_id) = self.loops.get(&loop_id).unwrap_or_else(|| {
+                    panic!("Found loop ID {:?} with no corresponding loop info", loop_id)
+                });
 
                 // If our current loop contains the entry, move on to the next entry. Otherwise
                 // move on to the next wider loop if there is one.

From e0e159e1aff6dfceca49402ffe92db1e48e2d6bc Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Mon, 6 Oct 2025 16:54:06 -0700
Subject: [PATCH 13/15] Add overview of relooper

---
 c2rust-transpile/src/cfg/mod.rs      |   1 +
 c2rust-transpile/src/cfg/relooper.rs | 121 ++++++++++++++++++++++++---
 2 files changed, 112 insertions(+), 10 deletions(-)

diff --git a/c2rust-transpile/src/cfg/mod.rs b/c2rust-transpile/src/cfg/mod.rs
index 270c640267..6b1146cfa7 100644
--- a/c2rust-transpile/src/cfg/mod.rs
+++ b/c2rust-transpile/src/cfg/mod.rs
@@ -14,6 +14,7 @@
 //!   - simplify that sequence of `Structure<Stmt>`s into another such sequence
 //!   - convert the `Vec<Structure<Stmt>>` back into a `Vec<Stmt>`
 //!
+//! See the [`relooper`] module for more details about the Relooper algorithm.
 
 use crate::c_ast::iterators::{DFExpr, SomeId};
 use crate::c_ast::CLabelId;
diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 730bdcfcf1..3d7ae5550b 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -1,13 +1,104 @@
-//! This modules handles converting a a control-flow graph `Cfg` into `Vec<Structure>`, optionally
-//! simplifying the latter.
+//! This module contains the relooper algorithm for creating structured control
+//! flow from a CFG.
+//!
+//! Relooper is an algorithm for converting an arbitrary, unstructured
+//! control-flow graph (CFG) into the structured control-flow constructs
+//! available in Rust. The original relooper algorithm was described in the
+//! [Emscripten paper][emscripten], which describes converting a CFG into
+//! JavaScript's structured control-flow constructs. The implementation of
+//! relooper used here is based on the original algorithm, with the addition of
+//! some heuristics and Rust-specific control-flow constructs.
+//!
+//! [emscripten]: https://dl.acm.org/doi/10.1145/2048147.2048224
 //!
 //! # Terminology
 //!
-//! The terms "label", "block", and "node" are sometimes used interchangeably.
-//! A label is the unique identifier for a block, and a block is a node in the
-//! control-flow graph. In some cases we're working directly with the basic
-//! blocks, but in many places when working with the control-flow graph we're
-//! dealing only with labels, and the blocks themselves are secondary.
+//! The terms "label", "block", and "node" are sometimes used interchangeably in
+//! this file. A label is the unique identifier for a block, and a block is a
+//! node in the control-flow graph. In some cases we're working directly with
+//! the basic blocks, but in many places when working with the CFG we're dealing
+//! only with labels, and the blocks themselves are secondary.
+//!
+//! # Relooper Algorithm
+//!
+//! The relooper algorithm works by recursively breaking down sections of the
+//! CFG into structured control-flow constructs, partitioning blocks into either
+//! `Simple` structures, `Loop` structures, or `Multiple` structures. At each
+//! step, we start with a set of basic blocks and information about which of
+//! those blocks act as entry points to this portion of the CFG. We then look
+//! for ways to break down the CFG into smaller sections that can be represented
+//! using structured control-flow constructs.
+//!
+//! If we have a single entry point, and there are no back edges to that entry,
+//! then we can generate a simple structure. `Simple` structures contain a
+//! single basic block, and represent a straight-line sequence of instructions.
+//! All remaining blocks are then recursively relooped, with the immediate
+//! successors of the entry becoming the new entries for the rest of that
+//! portion of the CFG.
+//!
+//! If we have entries with back edges to them, then we can generate a `Loop`
+//! structure. Any nodes that can reach the entry become part of the loop body,
+//! with any remaining nodes becoming the follow blocks for the loop. The loop's
+//! contents are then relooped into the loops's body, and the follow blocks get
+//! relooped to be the logic that follows the loop.
+//!
+//! If we have more than one entry, then we can generate a `Multiple` structure.
+//! These are effectively `match` statements, with each entry becoming an arm of
+//! the `match`. Blocks that are only reachable by one of the entries (including
+//! the entry itself) become the body blocks for that arm of the multiple, and
+//! any blocks reachable from more than one entry become the follow blocks. We
+//! then recursively reloop each of the branches of the multiple, and then
+//! reloop the follow blocks.
+//!
+//! Note that there are a lot of subtleties to how we choose to partition blocks
+//! into these structures. The logic in the relooper implementation contains
+//! thorough comments describing what we're doing at each step and why we are
+//! making the choices that we do. This module documentation covers some of
+//! them, but you'll need to read through the full algorithm to get all of the
+//! nuances.
+//!
+//! # Heuristics
+//!
+//! When reconstructing structured control flow from a CFG, there are often
+//! multiple valid ways to structure the graph. In order to produce Rust code
+//! that is as similar to the original C as possible, we have a couple of
+//! heuristics that use information from the original C code to guide the
+//! restructuring process.
+//!
+//! Before creating a loop, we first try to match a `Multiple` from the original
+//! C. During transpilation we preserve information about where there are
+//! branches in the C code along with which CFG nodes are part of those
+//! branches, which we can then look up based on the current set of entries. If
+//! we find that there is a `Multiple` in the original C that matches our
+//! current entries, and the structure of the CFG allows it, we can reproduce
+//! the control-flow from the original C. Doing this before creating a loop
+//! helps in cases where we have branches with multiple disjoint loops, since
+//! the loop analysis does not recognize disjoint loops and will always produce
+//! a single loop with a `Multiple` inside of it handling the bodies of what
+//! should be separate loops.
+//!
+//! When creating loops, we also make use of a similar heuristic that tries to
+//! recreate the loops that we see in the original C. When partitioning blocks
+//! into the loop's body, we first attempt to match an existing loop from the
+//! original C. Failing that, we fall back on a heuristic that tries to keep as
+//! many blocks as possible in the loop's body, even if they don't strictly
+//! belong there according to the original C structure.
+//! 
+//! # Simplification
+//! 
+//! After the relooper algorithm runs, we have an optional simplification pass
+//! that attempts to reduce the complexity of the generated control flow
+//! structures. This pass can help to eliminate unnecessary nesting and make the
+//! final output more readable. It applies two main simplifications:
+//! 
+//! - Merge cases in [`Switch`] terminators if they target the same label. That
+//!   means instead of having `1 => goto A, 2 => goto A, 3 => goto B`, we
+//!   instead get `1 | 2 => goto A, 3 => goto B`.
+//! - Inline `Multiple` structures into preceding `Simple` structures. When a
+//!   `Simple` structure with a `Switch` terminator is immediately followed by a
+//!   `Multiple`, the branches from the `Multiple` are inlined directly into the
+//!   `Switch` cases as `Nested` structures, eliminating the intermediate
+//!   `Multiple` and reducing nesting depth.
 
 use super::*;
 
@@ -360,8 +451,17 @@ impl RelooperState {
             (predecessor_map, strict_reachable_from)
         };
 
-        // Try to match an existing branch point (from the initial C). See `MultipleInfo` for more
-        // information on this.
+        // Try to match an existing branch point (from the initial C).
+        //
+        // We do this before creating a loop to better handle the cases where we have
+        // multiple entries that are part of disjoint loops. The loop analysis that
+        // comes next doesn't recognize disjoint loops and will always produce a single
+        // loop structure if there is at least one loop back to an entry. If we have
+        // multiple branches with disjoint loops, this means we might generate a single
+        // loop with a state machine inside it, merging all loop bodies together and
+        // resulting in suboptimal control flow. If the original C had multiple
+        // branches, this logic will recreate them, avoiding the problem of merging
+        // loops accidentally.
         let mut recognized_c_multiple = false;
         if let Some(ref multiple_info) = self.multiple_info {
             let entries_key = entries.iter().cloned().collect();
@@ -616,7 +716,8 @@ impl RelooperState {
         // This is done because of how we generate `match` statements from `Multiple`.
         // If all entries are handled, then we pull one case to be the `_ => { ... }`
         // default case in the generated `match`. This works because every entry will
-        // have its own `match` arm, so it's safe to have the `_` arm cover one of them.
+        // have its own `match` arm, so it's safe to have the `_` arm cover one of them,
+        // and avoids having a dead `_` case just to satisfy exhaustiveness.
         //
         // If we have unhandled entries, then we'll have entries not covered by the
         // `match`. In this case we want an empty `_ => {}` case, that way when we hit

From 070bd76b2a0be3d41e5863c3ba8813d69b38dee2 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Mon, 6 Oct 2025 17:25:17 -0700
Subject: [PATCH 14/15] Fix formatting

---
 c2rust-transpile/src/cfg/loops.rs    | 5 ++++-
 c2rust-transpile/src/cfg/relooper.rs | 6 +++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/c2rust-transpile/src/cfg/loops.rs b/c2rust-transpile/src/cfg/loops.rs
index 1ac69a4617..9e462ac116 100644
--- a/c2rust-transpile/src/cfg/loops.rs
+++ b/c2rust-transpile/src/cfg/loops.rs
@@ -181,7 +181,10 @@ impl<Lbl: Hash + Eq + Clone> LoopInfo<Lbl> {
         for entry in entries {
             loop {
                 let (in_loop, parent_id) = self.loops.get(&loop_id).unwrap_or_else(|| {
-                    panic!("Found loop ID {:?} with no corresponding loop info", loop_id)
+                    panic!(
+                        "Found loop ID {:?} with no corresponding loop info",
+                        loop_id,
+                    )
                 });
 
                 // If our current loop contains the entry, move on to the next entry. Otherwise
diff --git a/c2rust-transpile/src/cfg/relooper.rs b/c2rust-transpile/src/cfg/relooper.rs
index 3d7ae5550b..bb6dc3bddd 100644
--- a/c2rust-transpile/src/cfg/relooper.rs
+++ b/c2rust-transpile/src/cfg/relooper.rs
@@ -83,14 +83,14 @@
 //! original C. Failing that, we fall back on a heuristic that tries to keep as
 //! many blocks as possible in the loop's body, even if they don't strictly
 //! belong there according to the original C structure.
-//! 
+//!
 //! # Simplification
-//! 
+//!
 //! After the relooper algorithm runs, we have an optional simplification pass
 //! that attempts to reduce the complexity of the generated control flow
 //! structures. This pass can help to eliminate unnecessary nesting and make the
 //! final output more readable. It applies two main simplifications:
-//! 
+//!
 //! - Merge cases in [`Switch`] terminators if they target the same label. That
 //!   means instead of having `1 => goto A, 2 => goto A, 3 => goto B`, we
 //!   instead get `1 | 2 => goto A, 3 => goto B`.

From 1be52a6080891d074a4067d8f0ce6bc63d0d1925 Mon Sep 17 00:00:00 2001
From: Nicole LeGare <legaren@immunant.com>
Date: Tue, 7 Oct 2025 10:58:35 -0700
Subject: [PATCH 15/15] DON'T PANIC!!!

---
 c2rust-transpile/src/cfg/loops.rs | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/c2rust-transpile/src/cfg/loops.rs b/c2rust-transpile/src/cfg/loops.rs
index 9e462ac116..462c873a59 100644
--- a/c2rust-transpile/src/cfg/loops.rs
+++ b/c2rust-transpile/src/cfg/loops.rs
@@ -180,12 +180,13 @@ impl<Lbl: Hash + Eq + Clone> LoopInfo<Lbl> {
         // be widened.
         for entry in entries {
             loop {
-                let (in_loop, parent_id) = self.loops.get(&loop_id).unwrap_or_else(|| {
-                    panic!(
-                        "Found loop ID {:?} with no corresponding loop info",
-                        loop_id,
-                    )
-                });
+                // NOTE: We currently bail out in the case where there is no loop data
+                // corresponding to the current loop ID, but it's unclear if that's the right
+                // thing to do. In theory we should always have loop data corresponding to a
+                // loop ID, but the `nested_goto.c` test definitely ends up with a loop ID that
+                // doesn't have corresponding info. We should investigate why that happens and
+                // determine if it's valid or not.
+                let (in_loop, parent_id) = self.loops.get(&loop_id)?;
 
                 // If our current loop contains the entry, move on to the next entry. Otherwise
                 // move on to the next wider loop if there is one.