1 changed files with 60 additions and 41 deletions
--- a/src/merged_tree.rs
+++ b/src/merged_tree.rs
@ -8,6 +8,8 @@ use std::{

 use either::Either;
 use itertools::{EitherOrBoth, Itertools};
+use log::warn;
+use rustc_hash::FxHashSet;

 use crate::{
    ast::AstNode,
@ -258,55 +260,72 @@ impl<'a> MergedTree<'a> {
            // no nodes to force line-based fallback on
            return self;
        }
-
-        match self {
-            Self::ExactTree { node, .. } | Self::MixedTree { node, .. }
-                if nodes.contains(&node) =>
-            {
-                Self::line_based_local_fallback_for_revnode(node, class_mapping, settings)
+        // NOTE: This implement cycle detection. It's a temporary measure, introduced to mitigate
+        // a bug in comment bundling logic. See https://codeberg.org/mergiraf/mergiraf/issues/609
+        fn inner<'a>(
+            this: MergedTree<'a>,
+            nodes: &HashSet<Leader<'a>>,
+            class_mapping: &ClassMapping<'a>,
+            settings: &DisplaySettings,
+            seen_nodes: &mut FxHashSet<MergedTree<'a>>,
+        ) -> MergedTree<'a> {
+            if seen_nodes.contains(&this) {
+                warn!("detected a cycle in `force_line_based_fallback_on_specific_nodes`: {this}");
+                if cfg!(debug_assertions) {
+                    // panic!(); <- this currently triggers in our own test suite, so don't
+                    //              activate it for now
+                } else {
+                    return this;
+                }
+            } else {
+                seen_nodes.insert(this.clone());
            }
-            Self::ExactTree {
-                node, revisions, ..
-            } => {
-                let picked_revision = revisions.any();
-                let children = class_mapping
-                    .children_at_revision(&node, picked_revision)
-                    .expect("non-existent children for revision in revset of ExactTree");
-                let cloned_children: Vec<MergedTree<'a>> = children
-                    .into_iter()
-                    .map(|c| {
-                        Self::new_exact(c, revisions, class_mapping)
-                            .force_line_based_fallback_on_specific_nodes(
+            match this {
+                MergedTree::ExactTree { node, .. } | MergedTree::MixedTree { node, .. }
+                    if nodes.contains(&node) =>
+                {
+                    MergedTree::line_based_local_fallback_for_revnode(node, class_mapping, settings)
+                }
+                MergedTree::ExactTree {
+                    node, revisions, ..
+                } => {
+                    let picked_revision = revisions.any();
+                    let children = class_mapping
+                        .children_at_revision(&node, picked_revision)
+                        .expect("non-existent children for revision in revset of ExactTree");
+                    let cloned_children: Vec<MergedTree<'a>> = children
+                        .into_iter()
+                        .map(|c| {
+                            inner(
+                                MergedTree::new_exact(c, revisions, class_mapping),
                                nodes,
                                class_mapping,
                                settings,
+                                seen_nodes,
                            )
-                    })
-                    .collect();
-                if cloned_children
-                    .iter()
-                    .all(|child| matches!(child, Self::ExactTree { .. }))
-                {
-                    self
-                } else {
-                    Self::new_mixed(node, cloned_children)
+                        })
+                        .collect();
+                    if cloned_children
+                        .iter()
+                        .all(|child| matches!(child, MergedTree::ExactTree { .. }))
+                    {
+                        this
+                    } else {
+                        MergedTree::new_mixed(node, cloned_children)
+                    }
                }
+                MergedTree::MixedTree { node, children, .. } => {
+                    let cloned_children = children
+                        .into_iter()
+                        .map(|c| inner(c, nodes, class_mapping, settings, seen_nodes))
+                        .collect();
+                    MergedTree::new_mixed(node, cloned_children)
+                }
+                _ => this,
            }
-            Self::MixedTree { node, children, .. } => {
-                let cloned_children = children
-                    .into_iter()
-                    .map(|c| {
-                        c.force_line_based_fallback_on_specific_nodes(
-                            nodes,
-                            class_mapping,
-                            settings,
-                        )
-                    })
-                    .collect();
-                Self::new_mixed(node, cloned_children)
-            }
-            _ => self,
        }
+        let mut seen_nodes = Default::default();
+        inner(self, nodes, class_mapping, settings, &mut seen_nodes)
    }

    /// Checks if a particular node is contained in the result tree