Merge pull request tree-sitter#2788 from tree-sitter/subtree-compare-stack-overflow

Avoid using recursion in `subtree_compare`

Author: maxbrunsfeld

cli/src/tests/language_test.rs

@@ -62,34 +62,3 @@ fn test_lookahead_iterator_modifiable_only_by_mut() {
     let mut names = lookahead.iter_names();
     let _ = names.next();
 }
-
-/// It doesn't allowed to use lookahead iterator by shared ref:
-///     error[E0596]: cannot borrow `lookahead` as mutable, as it is not declared as mutable
-/// ```compile_fail
-/// use tree_sitter::{Parser, Language};
-/// let mut parser = Parser::new();
-/// let language = unsafe { Language::from_raw(std::ptr::null()) };
-/// let tree = parser.parse("", None).unwrap();
-/// let mut cursor = tree.walk();
-/// let next_state = cursor.node().next_parse_state();
-/// let lookahead = language.lookahead_iterator(next_state).unwrap();
-/// let _ = lookahead.next();
-/// ```
-
-/// It doesn't allowed to use lookahead names iterator by shared ref:
-///     error[E0596]: cannot borrow `names` as mutable, as it is not declared as mutable
-/// ```compile_fail
-/// use tree_sitter::{Parser, Language};
-/// let mut parser = Parser::new();
-/// let language = unsafe { Language::from_raw(std::ptr::null()) };
-/// let tree = parser.parse("", None).unwrap();
-/// let mut cursor = tree.walk();
-/// let next_state = cursor.node().next_parse_state();
-/// if let Some(mut lookahead) = language.lookahead_iterator(next_state) {
-///     let _ = lookahead.next();
-///     let names = lookahead.iter_names();
-///     let _ = names.next();
-/// }
-/// ```
-
-fn _dummy() {}

lib/src/parser.c

@@ -747,7 +747,7 @@ static bool ts_parser__select_tree(TSParser *self, Subtree left, Subtree right)
 
   if (ts_subtree_error_cost(left) > 0) return true;
 
-  int comparison = ts_subtree_compare(left, right);
+  int comparison = ts_subtree_compare(left, right, &self->tree_pool);
   switch (comparison) {
     case -1:
       LOG("select_earlier symbol:%s, over_symbol:%s", TREE_NAME(left), TREE_NAME(right));

lib/src/subtree.c

@@ -1,10 +1,11 @@
 #include <assert.h>
 #include <ctype.h>
-#include <limits.h>
+#include <stdint.h>
 #include <stdbool.h>
 #include <string.h>
 #include <stdio.h>
 #include "./alloc.h"
+#include "./array.h"
 #include "./atomic.h"
 #include "./subtree.h"
 #include "./length.h"
@@ -618,21 +619,30 @@ void ts_subtree_release(SubtreePool *pool, Subtree self) {
   }
 }
 
-int ts_subtree_compare(Subtree left, Subtree right) {
-  if (ts_subtree_symbol(left) < ts_subtree_symbol(right)) return -1;
-  if (ts_subtree_symbol(right) < ts_subtree_symbol(left)) return 1;
-  if (ts_subtree_child_count(left) < ts_subtree_child_count(right)) return -1;
-  if (ts_subtree_child_count(right) < ts_subtree_child_count(left)) return 1;
-  for (uint32_t i = 0, n = ts_subtree_child_count(left); i < n; i++) {
-    Subtree left_child = ts_subtree_children(left)[i];
-    Subtree right_child = ts_subtree_children(right)[i];
-    switch (ts_subtree_compare(left_child, right_child)) {
-      case -1: return -1;
-      case 1: return 1;
-      default: break;
+int ts_subtree_compare(Subtree left, Subtree right, SubtreePool *pool) {
+  array_push(&pool->tree_stack, ts_subtree_to_mut_unsafe(left));
+  array_push(&pool->tree_stack, ts_subtree_to_mut_unsafe(right));
+
+  int result = 0;
+  while (result == 0 && pool->tree_stack.size > 0) {
+    right = ts_subtree_from_mut(array_pop(&pool->tree_stack));
+    left = ts_subtree_from_mut(array_pop(&pool->tree_stack));
+
+    if (ts_subtree_symbol(left) < ts_subtree_symbol(right)) result = -1;
+    if (ts_subtree_symbol(right) < ts_subtree_symbol(left)) result = 1;
+    if (ts_subtree_child_count(left) < ts_subtree_child_count(right)) result = -1;
+    if (ts_subtree_child_count(right) < ts_subtree_child_count(left)) result = 1;
+
+    for (uint32_t i = ts_subtree_child_count(left); i > 0; i--) {
+      Subtree left_child = ts_subtree_children(left)[i - 1];
+      Subtree right_child = ts_subtree_children(right)[i - 1];
+      array_push(&pool->tree_stack, ts_subtree_to_mut_unsafe(left_child));
+      array_push(&pool->tree_stack, ts_subtree_to_mut_unsafe(right_child));
     }
   }
-  return 0;
+
+  array_clear(&pool->tree_stack);
+  return result;
 }
 
 static inline void ts_subtree_set_has_changes(MutableSubtree *self) {

lib/src/subtree.h

@@ -200,7 +200,7 @@ Subtree ts_subtree_new_missing_leaf(SubtreePool *, TSSymbol, Length, uint32_t, c
 MutableSubtree ts_subtree_make_mut(SubtreePool *, Subtree);
 void ts_subtree_retain(Subtree);
 void ts_subtree_release(SubtreePool *, Subtree);
-int ts_subtree_compare(Subtree, Subtree);
+int ts_subtree_compare(Subtree, Subtree, SubtreePool *);
 void ts_subtree_set_symbol(MutableSubtree *, TSSymbol, const TSLanguage *);
 void ts_subtree_summarize(MutableSubtree, const Subtree *, uint32_t, const TSLanguage *);
 void ts_subtree_summarize_children(MutableSubtree, const TSLanguage *);