Move cache retrieval on forwards scans to bidi_resolve_brackets.

1 files changed, 61 insertions, 89 deletions

diff --git a/src/bidi.c b/src/bidi.c
index abcfa2a9b90..8f996eb6539 100644
--- a/src/bidi.c
+++ b/src/bidi.c
@@ -800,26 +800,22 @@ bidi_cache_iterator_state (struct bidi_it *bidi_it, bool resolved,
 
 /* Look for a cached iterator state that corresponds to CHARPOS.  If
    found, copy the cached state into BIDI_IT and return the type of
-   the cached entry.  If not found, return UNKNOWN_BT.  NEUTRALS_OK
-   non-zero means it is OK to return cached state for neutral
-   characters that have no valid next_for_neutral member, and
-   therefore cannot be resolved.  This can happen if the state was
-   cached before it was resolved in bidi_resolve_neutral.  */
+   the cached entry.  If not found, return UNKNOWN_BT.  RESOLVED_ONLY
+   zero means it is OK to return cached states tyhat were not fully
+   resolved yet.  This can happen if the state was cached before it
+   was resolved in bidi_resolve_neutral.  */
 static bidi_type_t
-bidi_cache_find (ptrdiff_t charpos, bool neutrals_ok, struct bidi_it *bidi_it)
+bidi_cache_find (ptrdiff_t charpos, bool resolved_only, struct bidi_it *bidi_it)
 {
   ptrdiff_t i = bidi_cache_search (charpos, -1, bidi_it->scan_dir);
 
   if (i >= bidi_cache_start
-      && (neutrals_ok
-          /* Callers that don't want to resolve neutrals (and set
-             neutrals_ok = false) need to be sure that there's enough
-             info in the cached state to resolve the neutrals and
-             isolates, and if not, they don't want the cached state.  */
-          || !(bidi_cache[i].resolved_level == -1
-               && (bidi_get_category (bidi_cache[i].type) == NEUTRAL
-                   || bidi_isolate_fmt_char (bidi_cache[i].type))
-               && bidi_cache[i].next_for_neutral.type == UNKNOWN_BT)))
+      && (!resolved_only
+          /* Callers that want only fully resolved states (and set
+             resolved_only = true) need to be sure that there's enough
+             info in the cached state to return the state as final,
+             and if not, they don't want the cached state.  */
+          || bidi_cache[i].resolved_level >= 0))
     {
       bidi_dir_t current_scan_dir = bidi_it->scan_dir;
 
@@ -2522,7 +2518,7 @@ bidi_find_bracket_pairs (struct bidi_it *bidi_it)
 
       /* Restore bidi_it from the cache, which should have the bracket
          resolution members set as determined by the above loop.  */
-      type = bidi_cache_find (saved_it.charpos, 1, bidi_it);
+      type = bidi_cache_find (saved_it.charpos, 0, bidi_it);
       eassert (type == NEUTRAL_ON);
     }
 
@@ -2530,7 +2526,8 @@ bidi_find_bracket_pairs (struct bidi_it *bidi_it)
 }
 
 static void
-bidi_record_prev_for_neutral (struct bidi_saved_info *info, int level)
+bidi_record_type_for_neutral (struct bidi_saved_info *info, int level,
+                              bool nextp)
 {
   int idx;
 
@@ -2538,16 +2535,16 @@ bidi_record_prev_for_neutral (struct bidi_saved_info *info, int level)
     {
       int lev = bidi_cache[idx].level_stack[bidi_cache[idx].stack_idx].level;
 
-      if (bidi_cache[idx].type_after_wn == NEUTRAL_B)
-        return;         /* no cache slot to update */
       if (lev <= level)
         {
           eassert (lev == level);
-          bidi_cache[idx].prev_for_neutral = *info;
+          if (nextp)
+            bidi_cache[idx].next_for_neutral = *info;
+          else
+            bidi_cache[idx].prev_for_neutral = *info;
           break;
         }
     }
-  eassert (idx < bidi_cache_idx);
 }
 
 static bidi_type_t
@@ -2557,19 +2554,24 @@ bidi_resolve_brackets (struct bidi_it *bidi_it)
   bool resolve_bracket = false;
   bidi_type_t type = UNKNOWN_BT;
   int ch;
-  struct bidi_saved_info tem_info;
+  struct bidi_saved_info prev_for_neutral, next_for_neutral;
 
   /* Record the prev_for_neutral type either from the previous
      character, if it was a strong or AN/EN, or from the
      prev_for_neutral information recorded previously.  */
   if (bidi_it->type == STRONG_L || bidi_it->type == STRONG_R
       || bidi_it->type == WEAK_AN || bidi_it->type == WEAK_EN)
-    bidi_remember_char (&tem_info, bidi_it, 1);
+    bidi_remember_char (&prev_for_neutral, bidi_it, 1);
+  else
+    prev_for_neutral = bidi_it->prev_for_neutral;
+  /* Record the next_for_neutral type information.  */
+  if (bidi_it->next_for_neutral.charpos > bidi_it->charpos)
+    next_for_neutral = bidi_it->next_for_neutral;
   else
-    tem_info = bidi_it->prev_for_neutral;
+    next_for_neutral.charpos = -1;
   if (!bidi_it->first_elt)
     {
-      type = bidi_cache_find (bidi_it->charpos + bidi_it->nchars, 1, bidi_it);
+      type = bidi_cache_find (bidi_it->charpos + bidi_it->nchars, 0, bidi_it);
       ch = bidi_it->ch;
     }
   if (type == UNKNOWN_BT)
@@ -2580,14 +2582,18 @@ bidi_resolve_brackets (struct bidi_it *bidi_it)
     }
   else
     {
+      eassert (bidi_it->resolved_level == -1);
       /* If the cached state shows an increase of embedding level due
          to an isolate initiator, we need to update the 1st cached
          state of the next run of the current isolating sequence with
-         the prev_for_neutral information, so that it will be picked
-         up when we advance to that next run.  */
+         the prev_for_neutral and next_for_neutral information, so
+         that it will be picked up when we advance to that next run.  */
       if (bidi_it->level_stack[bidi_it->stack_idx].level > prev_level
           && bidi_it->level_stack[bidi_it->stack_idx].isolate_status)
-        bidi_record_prev_for_neutral (&tem_info, prev_level);
+        {
+          bidi_record_type_for_neutral (&prev_for_neutral, prev_level, 0);
+          bidi_record_type_for_neutral (&next_for_neutral, prev_level, 1);
+        }
       if (type == NEUTRAL_ON
           && bidi_paired_bracket_type (ch) == BIDI_BRACKET_OPEN)
         {
@@ -2606,10 +2612,15 @@ bidi_resolve_brackets (struct bidi_it *bidi_it)
                 resolve_bracket = true;
             }
         }
-      /* Keep track of the prev_for_neutral type, needed for resolving
-         brackets below and for resolving neutrals in bidi_resolve_neutral.  */
+      /* Keep track of the prev_for_neutral and next_for_neutral
+         types, needed for resolving brackets below and for resolving
+         neutrals in bidi_resolve_neutral.  */
       if (bidi_it->level_stack[bidi_it->stack_idx].level == prev_level)
-        bidi_it->prev_for_neutral = tem_info;
+        {
+          bidi_it->prev_for_neutral = prev_for_neutral;
+          if (next_for_neutral.charpos > 0)
+            bidi_it->next_for_neutral = next_for_neutral;
+        }
     }
 
   /* If needed, resolve the bracket type according to N0.  */
@@ -2694,9 +2705,18 @@ bidi_resolve_neutral (struct bidi_it *bidi_it)
       || (type == WEAK_BN && bidi_explicit_dir_char (bidi_it->ch)))
     {
       if (bidi_it->next_for_neutral.type != UNKNOWN_BT)
-        type = bidi_resolve_neutral_1 (bidi_it->prev_for_neutral.type,
-                                       bidi_it->next_for_neutral.type,
-                                       current_level);
+        {
+          /* Make sure the data for resolving neutrals we are
+             about to use is valid.  */
+          eassert (bidi_it->next_for_neutral.charpos > bidi_it->charpos
+                   /* PDI defines an eos, so it's OK for it to
+                      serve as its own next_for_neutral.  */
+                   || (bidi_it->next_for_neutral.charpos == bidi_it->charpos
+                       && bidi_it->type == PDI));
+          type = bidi_resolve_neutral_1 (bidi_it->prev_for_neutral.type,
+                                         bidi_it->next_for_neutral.type,
+                                         current_level);
+        }
       /* The next two "else if" clauses are shortcuts for the
          important special case when we have a long sequence of
          neutral or WEAK_BN characters, such as whitespace or nulls or
@@ -2892,16 +2912,13 @@ bidi_level_of_next_char (struct bidi_it *bidi_it)
         }
     }
 
-  /* Perhaps the character we want is already cached.  If it is, the
-     call to bidi_cache_find below will return a type other than
-     UNKNOWN_BT.  */
+  /* Perhaps the character we want is already cached s fully resolved.
+     If it is, the call to bidi_cache_find below will return a type
+     other than UNKNOWN_BT.  */
   if (bidi_cache_idx > bidi_cache_start && !bidi_it->first_elt)
     {
       int bob = ((bidi_it->string.s || STRINGP (bidi_it->string.lstring))
                  ? 0 : 1);
-      bidi_type_t prev_type = bidi_it->type;
-      bidi_type_t type_for_neutral = bidi_it->next_for_neutral.type;
-      ptrdiff_t pos_for_neutral = bidi_it->next_for_neutral.charpos;
 
       if (bidi_it->scan_dir > 0)
         {
@@ -2916,57 +2933,12 @@ bidi_level_of_next_char (struct bidi_it *bidi_it)
            cached at the beginning of the iteration.  */
         next_char_pos = bidi_it->charpos - 1;
       if (next_char_pos >= bob - 1)
-        type = bidi_cache_find (next_char_pos, 0, bidi_it);
-
-      /* For a sequence of BN and NI, copy the type from the previous
-         character.  This is because the loop in bidi_resolve_neutral
-         that handles such sequences caches the characters it
-         traverses, but does not (and cannot) store the
-         next_for_neutral member for them, because it is only known
-         when the loop ends.  So when we find them in the cache, their
-         type needs to be updated, but we don't have next_for_neutral
-         to do that.  However, whatever type is resolved as result of
-         that loop, it will be the same for all the traversed
-         characters, by virtue of N1 and N2.  */
-      if (type == WEAK_BN && bidi_it->scan_dir > 0
-          && bidi_explicit_dir_char (bidi_it->ch)
-          && type_for_neutral != UNKNOWN_BT
-          && bidi_it->charpos < pos_for_neutral)
-        {
-          type = prev_type;
-          eassert (type != UNKNOWN_BT);
-        }
+        type = bidi_cache_find (next_char_pos, 1, bidi_it);
       if (type != UNKNOWN_BT)
         {
-          /* If resolved_level is -1, it means this state was cached
-             before it was completely resolved, so we cannot return
-             it.  */
-          if (bidi_it->resolved_level != -1)
-            {
-              eassert (bidi_it->resolved_level >= 0);
-              return bidi_it->resolved_level;
-            }
-          else
-            {
-              level = bidi_it->level_stack[bidi_it->stack_idx].level;
-              if (bidi_get_category (type) == NEUTRAL
-                  || bidi_isolate_fmt_char (type))
-                {
-                  /* Make sure the data for resolving neutrals we are
-                     about to use is valid.  */
-                  if (bidi_it->next_for_neutral.charpos < bidi_it->charpos
-                      /* PDI defines an eos, so it's OK for it to
-                         serve as its own next_for_neutral.  */
-                      || (bidi_it->next_for_neutral.charpos == bidi_it->charpos
-                          && bidi_it->type != PDI)
-                      || bidi_it->next_for_neutral.type == UNKNOWN_BT)
-                    emacs_abort ();
-
-                  type = bidi_resolve_neutral_1 (bidi_it->prev_for_neutral.type,
-                                                 bidi_it->next_for_neutral.type,
-                                                 level);
-                }
-            }
+          /* We asked the cache for fully resolved states.  */
+          eassert (bidi_it->resolved_level >= 0);
+          return bidi_it->resolved_level;
         }
     }