Allow empty external tokens during err recovery if they change the scanner's state

Author: maxbrunsfeld

lib/src/parser.c

@@ -408,6 +408,8 @@ static Subtree ts_parser__lex(
   Length error_start_position = length_zero();
   Length error_end_position = length_zero();
   uint32_t lookahead_end_byte = 0;
+  uint32_t external_scanner_state_len = 0;
+  bool external_scanner_state_changed = false;
   ts_lexer_reset(&self->lexer, start_position);
 
   for (;;) {
@@ -429,22 +431,32 @@ static Subtree ts_parser__lex(
       );
       ts_lexer_finish(&self->lexer, &lookahead_end_byte);
 
-      // Zero-length external tokens are generally allowed, but they're not
-      // allowed right after a syntax error. This is for two reasons:
-      // 1. After a syntax error, the lexer is looking for any possible token,
-      //    as opposed to the specific set of tokens that are valid in some
-      //    parse state. In this situation, it's very easy for an external
-      //    scanner to produce unwanted zero-length tokens.
-      // 2. The parser sometimes inserts *missing* tokens to recover from
-      //    errors. These tokens are also zero-length. If we allow more
-      //    zero-length tokens to be created after missing tokens, it
-      //    can lead to infinite loops. Forbidding zero-length tokens
-      //    right at the point of error recovery is a conservative strategy
-      //    for preventing this kind of infinite loop.
-      if (found_token && (
-        self->lexer.token_end_position.bytes > current_position.bytes ||
-        (!error_mode && ts_stack_has_advanced_since_error(self->stack, version))
-      )) {
+      if (found_token) {
+        external_scanner_state_len = self->language->external_scanner.serialize(
+          self->external_scanner_payload,
+          self->lexer.debug_buffer
+        );
+        external_scanner_state_changed = !ts_external_scanner_state_eq(
+          ts_subtree_external_scanner_state(external_token),
+          self->lexer.debug_buffer,
+          external_scanner_state_len
+        );
+
+        // When recovering from an error, ignore any zero-length external tokens
+        // unless they have changed the external scanner's state. This helps to
+        // avoid infinite loops which could otherwise occur, because the lexer is
+        // looking for any possible token, instead of looking for the specific set of
+        // tokens that are valid in some parse state.
+        if (
+          self->lexer.token_end_position.bytes == current_position.bytes &&
+          (error_mode || !ts_stack_has_advanced_since_error(self->stack, version)) &&
+          !external_scanner_state_changed
+        ) {
+          found_token = false;
+        }
+      }
+
+      if (found_token) {
         found_external_token = true;
         called_get_column = self->lexer.did_get_column;
         break;
@@ -549,15 +561,13 @@ static Subtree ts_parser__lex(
     );
 
     if (found_external_token) {
-      unsigned length = self->language->external_scanner.serialize(
-        self->external_scanner_payload,
-        self->lexer.debug_buffer
-      );
+      MutableSubtree mut_result = ts_subtree_to_mut_unsafe(result);
       ts_external_scanner_state_init(
-        &((SubtreeHeapData *)result.ptr)->external_scanner_state,
+        &mut_result.ptr->external_scanner_state,
         self->lexer.debug_buffer,
-        length
+        external_scanner_state_len
       );
+      mut_result.ptr->has_external_scanner_state_change = external_scanner_state_changed;
     }
   }
 
@@ -1199,6 +1209,15 @@ static void ts_parser__recover(
     return;
   }
 
+  if (
+    did_recover &&
+    ts_subtree_has_external_scanner_state_change(lookahead)
+  ) {
+    ts_stack_halt(self->stack, version);
+    ts_subtree_release(&self->tree_pool, lookahead);
+    return;
+  }
+
   // If the parser is still in the error state at the end of the file, just wrap everything
   // in an ERROR node and terminate.
   if (ts_subtree_is_eof(lookahead)) {
@@ -1929,6 +1948,7 @@ TSTree *ts_parser_parse(
     }
   } while (version_count != 0);
 
+  assert(self->finished_tree.ptr);
   ts_subtree_balance(self->finished_tree, &self->tree_pool, self->language);
   LOG("done");
   LOG_TREE(self->finished_tree);

lib/src/subtree.c

@@ -21,8 +21,6 @@ typedef struct {
 #define TS_MAX_INLINE_TREE_LENGTH UINT8_MAX
 #define TS_MAX_TREE_POOL_SIZE 32
 
-static const ExternalScannerState empty_state = {{.short_data = {0}}, .length = 0};
-
 // ExternalScannerState
 
 void ts_external_scanner_state_init(ExternalScannerState *self, const char *data, unsigned length) {
@@ -58,11 +56,10 @@ const char *ts_external_scanner_state_data(const ExternalScannerState *self) {
   }
 }
 
-bool ts_external_scanner_state_eq(const ExternalScannerState *a, const ExternalScannerState *b) {
-  return a == b || (
-    a->length == b->length &&
-    !memcmp(ts_external_scanner_state_data(a), ts_external_scanner_state_data(b), a->length)
-  );
+bool ts_external_scanner_state_eq(const ExternalScannerState *a, const char *buffer, unsigned length) {
+  return
+    a->length == length &&
+    memcmp(ts_external_scanner_state_data(a), buffer, length) == 0;
 }
 
 // SubtreeArray
@@ -214,6 +211,7 @@ Subtree ts_subtree_new_leaf(
       .fragile_right = false,
       .has_changes = false,
       .has_external_tokens = has_external_tokens,
+      .has_external_scanner_state_change = false,
       .depends_on_column = depends_on_column,
       .is_missing = false,
       .is_keyword = is_keyword,
@@ -381,6 +379,7 @@ void ts_subtree_summarize_children(
   self.ptr->node_count = 1;
   self.ptr->has_external_tokens = false;
   self.ptr->depends_on_column = false;
+  self.ptr->has_external_scanner_state_change = false;
   self.ptr->dynamic_precedence = 0;
 
   uint32_t structural_index = 0;
@@ -398,6 +397,10 @@ void ts_subtree_summarize_children(
       self.ptr->depends_on_column = true;
     }
 
+    if (ts_subtree_has_external_scanner_state_change(child)) {
+      self.ptr->has_external_scanner_state_change = true;
+    }
+
     if (i == 0) {
       self.ptr->padding = ts_subtree_padding(child);
       self.ptr->size = ts_subtree_size(child);
@@ -521,6 +524,7 @@ MutableSubtree ts_subtree_new_node(
     .visible = metadata.visible,
     .named = metadata.named,
     .has_changes = false,
+    .has_external_scanner_state_change = false,
     .fragile_left = fragile,
     .fragile_right = fragile,
     .is_keyword = false,
@@ -1024,14 +1028,26 @@ void ts_subtree_print_dot_graph(Subtree self, const TSLanguage *language, FILE *
   fprintf(f, "}\n");
 }
 
-bool ts_subtree_external_scanner_state_eq(Subtree self, Subtree other) {
-  const ExternalScannerState *state1 = &empty_state;
-  const ExternalScannerState *state2 = &empty_state;
-  if (self.ptr && ts_subtree_has_external_tokens(self) && !self.ptr->child_count) {
-    state1 = &self.ptr->external_scanner_state;
-  }
-  if (other.ptr && ts_subtree_has_external_tokens(other) && !other.ptr->child_count) {
-    state2 = &other.ptr->external_scanner_state;
+const ExternalScannerState *ts_subtree_external_scanner_state(Subtree self) {
+  static const ExternalScannerState empty_state = {{.short_data = {0}}, .length = 0};
+  if (
+    self.ptr &&
+    !self.data.is_inline &&
+    self.ptr->has_external_tokens &&
+    self.ptr->child_count == 0
+  ) {
+    return &self.ptr->external_scanner_state;
+  } else {
+    return &empty_state;
   }
-  return ts_external_scanner_state_eq(state1, state2);
+}
+
+bool ts_subtree_external_scanner_state_eq(Subtree a, Subtree b) {
+  const ExternalScannerState *state_a = ts_subtree_external_scanner_state(a);
+  const ExternalScannerState *state_b = ts_subtree_external_scanner_state(b);
+  return ts_external_scanner_state_eq(
+    state_a,
+    ts_external_scanner_state_data(state_b),
+    state_b->length
+  );
 }

lib/src/subtree.h

@@ -114,7 +114,7 @@ typedef struct {
   Length size;
   uint32_t lookahead_bytes;
   uint32_t error_cost;
-  uint32_t child_count;
+  uint16_t child_count;
   TSSymbol symbol;
   TSStateId parse_state;
 
@@ -125,6 +125,7 @@ typedef struct {
   bool fragile_right : 1;
   bool has_changes : 1;
   bool has_external_tokens : 1;
+  bool has_external_scanner_state_change : 1;
   bool depends_on_column: 1;
   bool is_missing : 1;
   bool is_keyword : 1;
@@ -135,8 +136,8 @@ typedef struct {
       uint32_t visible_child_count;
       uint32_t named_child_count;
       uint32_t node_count;
-      uint32_t repeat_depth;
       int32_t dynamic_precedence;
+      uint16_t repeat_depth;
       uint16_t production_id;
       struct {
         TSSymbol symbol;
@@ -174,6 +175,8 @@ typedef struct {
 
 void ts_external_scanner_state_init(ExternalScannerState *, const char *, unsigned);
 const char *ts_external_scanner_state_data(const ExternalScannerState *);
+bool ts_external_scanner_state_eq(const ExternalScannerState *a, const char *, unsigned);
+void ts_external_scanner_state_delete(ExternalScannerState *self);
 
 void ts_subtree_array_copy(SubtreeArray, SubtreeArray *);
 void ts_subtree_array_clear(SubtreePool *, SubtreeArray *);
@@ -206,6 +209,7 @@ Subtree ts_subtree_edit(Subtree, const TSInputEdit *edit, SubtreePool *);
 char *ts_subtree_string(Subtree, const TSLanguage *, bool include_all);
 void ts_subtree_print_dot_graph(Subtree, const TSLanguage *, FILE *);
 Subtree ts_subtree_last_external_token(Subtree);
+const ExternalScannerState *ts_subtree_external_scanner_state(Subtree self);
 bool ts_subtree_external_scanner_state_eq(Subtree, Subtree);
 
 #define SUBTREE_GET(self, name) (self.data.is_inline ? self.data.name : self.ptr->name)
@@ -331,6 +335,10 @@ static inline bool ts_subtree_has_external_tokens(Subtree self) {
   return self.data.is_inline ? false : self.ptr->has_external_tokens;
 }
 
+static inline bool ts_subtree_has_external_scanner_state_change(Subtree self) {
+  return self.data.is_inline ? false : self.ptr->has_external_scanner_state_change;
+}
+
 static inline bool ts_subtree_depends_on_column(Subtree self) {
   return self.data.is_inline ? false : self.ptr->depends_on_column;
 }

test/fixtures/error_corpus/python_errors.txt

@@ -1,29 +1,76 @@
-==========================================
-errors in if statements
-==========================================
+=============================================
+incomplete condition in if statement
+=============================================
 
 if a is:
-    print b
-    print c
+  print b
+  print c
+print d
 
 ---
 
 (module
-  (if_statement (identifier) (ERROR) (block
-    (print_statement (identifier))
-    (print_statement (identifier)))))
+  (if_statement
+    condition: (identifier)
+    (ERROR)
+    consequence: (block
+      (print_statement argument: (identifier))
+      (print_statement argument: (identifier))))
+  (print_statement argument: (identifier)))
 
 ==========================================
-errors in function definitions
+extra colon in function definition
 ==========================================
 
 def a()::
   b
   c
+d
+
+---
+
+(module
+  (function_definition
+    name: (identifier)
+    parameters: (parameters)
+    (ERROR)
+    body: (block
+      (expression_statement (identifier))
+      (expression_statement (identifier))))
+  (expression_statement (identifier)))
+
+========================================================
+incomplete if statement in function definition
+========================================================
+
+def a():
+  if a
+
+---
+
+(module
+  (function_definition
+    name: (identifier)
+    parameters: (parameters)
+    (ERROR (identifier))
+    body: (block)))
+
+========================================================
+incomplete expression before triple-quoted string
+========================================================
+
+def a():
+  b.
+  """
+  c
+  """
 
 ---
 
 (module
-  (function_definition (identifier) (parameters) (ERROR) (block
-    (expression_statement (identifier))
-    (expression_statement (identifier)))))
+  (function_definition
+    name: (identifier)
+    parameters: (parameters)
+    (ERROR (identifier))
+    body: (block
+      (expression_statement (string)))))