Merge pull request #15422 from bfredl/syn_name2id_go_brr

feat(highlights): some improvements and perf fixes

runtime/doc/vim_diff.txt

@@ -161,6 +161,7 @@ Commands:
   |:drop| is always available
   |:Man| is available by default, with many improvements such as completion
   |:sign-define| accepts a `numhl` argument, to highlight the line number
+  |:match| can be invoked before highlight group is defined
 
 Events:
   |Signal|
@@ -178,6 +179,7 @@ Functions:
   |msgpackdump()|, |msgpackparse()| provide msgpack de/serialization
   |stdpath()|
   |system()|, |systemlist()| can run {cmd} directly (without 'shell')
+  |matchadd()| can be called before highlight group is defined
 
 Highlight groups:
   |highlight-blend| controls blend level for a highlight group

src/nvim/buffer.c

@@ -4253,7 +4253,7 @@ int build_stl_str_hl(
       if (*fmt_p == '#') {
         stl_items[curitem].type = Highlight;
         stl_items[curitem].start = out_p;
-        stl_items[curitem].minwid = -syn_namen2id(t, (int)(fmt_p - t));
+        stl_items[curitem].minwid = -syn_name2id_len(t, (size_t)(fmt_p - t));
         curitem++;
         fmt_p++;
       }

src/nvim/main.c

@@ -166,6 +166,7 @@ void early_init(mparm_T *paramp)
   init_path(argv0 ? argv0 : "nvim");
   init_normal_cmds();   // Init the table of Normal mode commands.
   highlight_init();
+  syntax_init();
 
 #ifdef WIN32
   OSVERSIONINFO ovi;

src/nvim/map.c

@@ -194,6 +194,7 @@ static inline bool ColorKey_eq(ColorKey ae1, ColorKey ae2)
 
 MAP_IMPL(int, int, DEFAULT_INITIALIZER)
 MAP_IMPL(cstr_t, ptr_t, DEFAULT_INITIALIZER)
+MAP_IMPL(cstr_t, int, DEFAULT_INITIALIZER)
 MAP_IMPL(ptr_t, ptr_t, DEFAULT_INITIALIZER)
 MAP_IMPL(uint64_t, ptr_t, DEFAULT_INITIALIZER)
 MAP_IMPL(uint64_t, ssize_t, SSIZE_INITIALIZER)

src/nvim/map.h

@@ -36,6 +36,7 @@
 //
 MAP_DECLS(int, int)
 MAP_DECLS(cstr_t, ptr_t)
+MAP_DECLS(cstr_t, int)
 MAP_DECLS(ptr_t, ptr_t)
 MAP_DECLS(uint64_t, ptr_t)
 MAP_DECLS(uint64_t, ssize_t)

src/nvim/syntax.c

@@ -53,9 +53,9 @@ static bool did_syntax_onoff = false;
 /// Structure that stores information about a highlight group.
 /// The ID of a highlight group is also called group ID.  It is the index in
 /// the highlight_ga array PLUS ONE.
-struct hl_group {
+typedef struct hl_group {
   char_u      *sg_name;         ///< highlight group name
-  char_u      *sg_name_u;       ///< uppercase of sg_name
+  char        *sg_name_u;       ///< uppercase of sg_name
   bool sg_cleared;              ///< "hi clear" was used
   int sg_attr;                  ///< Screen attr @see ATTR_ENTRY
   int sg_link;                  ///< link to this highlight group ID
@@ -80,7 +80,7 @@ struct hl_group {
   char *sg_rgb_sp_name;         ///< RGB special color name
 
   int sg_blend;                 ///< blend level (0-100 inclusive), -1 if unset
-};
+} HlGroup;
 
 /// \addtogroup SG_SET
 /// @{
@@ -91,6 +91,7 @@ struct hl_group {
 
 // builtin |highlight-groups|
 static garray_T highlight_ga = GA_EMPTY_INIT_VALUE;
+Map(cstr_t, int) *highlight_unames;
 
 static inline struct hl_group * HL_TABLE(void)
 {
@@ -384,6 +385,11 @@ static int current_line_id = 0;             // unique number for current line
 static int syn_time_on = FALSE;
 # define IF_SYN_TIME(p) (p)
 
+void syntax_init(void)
+{
+  highlight_unames = map_new(cstr_t, int)();
+}
+
 // Set the timeout used for syntax highlighting.
 // Use NULL to reset, no timeout.
 void syn_set_timeout(proftime_T *tm)
@@ -3372,7 +3378,7 @@ static void syn_cmd_clear(exarg_T *eap, int syncing)
           XFREE_CLEAR(SYN_CLSTR(curwin->w_s)[scl_id].scl_list);
         }
       } else {
-        id = syn_namen2id(arg, (int)(arg_end - arg));
+        id = syn_name2id_len(arg, (int)(arg_end - arg));
         if (id == 0) {
           EMSG2(_(e_nogroup), arg);
           break;
@@ -3542,7 +3548,7 @@ syn_cmd_list(
         else
           syn_list_cluster(id - SYNID_CLUSTER);
       } else {
-        int id = syn_namen2id(arg, (int)(arg_end - arg));
+        int id = syn_name2id_len(arg, (int)(arg_end - arg));
         if (id == 0) {
           EMSG2(_(e_nogroup), arg);
         } else {
@@ -6635,7 +6641,7 @@ void do_highlight(const char *line, const bool forceit, const bool init)
 
   // ":highlight {group-name}": list highlighting for one group.
   if (!doclear && !dolink && ends_excmd((uint8_t)(*linep))) {
-    id = syn_namen2id((const char_u *)line, (int)(name_end - line));
+    id = syn_name2id_len((const char_u *)line, (int)(name_end - line));
     if (id == 0) {
       emsgf(_("E411: highlight group not found: %s"), line);
     } else {
@@ -7113,6 +7119,7 @@ void free_highlight(void)
     xfree(HL_TABLE()[i].sg_name_u);
   }
   ga_clear(&highlight_ga);
+  map_free(cstr_t, int)(highlight_unames);
 }
 
 #endif
@@ -7470,26 +7477,35 @@ static void set_hl_attr(int idx)
   }
 }
 
+int syn_name2id(const char_u *name)
+  FUNC_ATTR_NONNULL_ALL
+{
+  return syn_name2id_len(name, STRLEN(name));
+}
+
 /// Lookup a highlight group name and return its ID.
 ///
 /// @param highlight name e.g. 'Cursor', 'Normal'
 /// @return the highlight id, else 0 if \p name does not exist
-int syn_name2id(const char_u *name)
+int syn_name2id_len(const char_u *name, size_t len)
   FUNC_ATTR_NONNULL_ALL
 {
-  int i;
-  char_u name_u[200];
+  char name_u[201];
 
-  /* Avoid using stricmp() too much, it's slow on some systems */
-  /* Avoid alloc()/free(), these are slow too.  ID names over 200 chars
-   * don't deserve to be found! */
-  STRLCPY(name_u, name, 200);
-  vim_strup(name_u);
-  for (i = highlight_ga.ga_len; --i >= 0; )
-    if (HL_TABLE()[i].sg_name_u != NULL
-        && STRCMP(name_u, HL_TABLE()[i].sg_name_u) == 0)
-      break;
-  return i + 1;
+  if (len == 0 || len > 200) {
+    // ID names over 200 chars don't deserve to be found!
+    return 0;
+  }
+
+  // Avoid using stricmp() too much, it's slow on some systems */
+  // Avoid alloc()/free(), these are slow too.
+  memcpy(name_u, name, len);
+  name_u[len] = '\0';
+  vim_strup((char_u *)name_u);
+
+  // map_get(..., int) returns 0 when no key is present, which is
+  // the expected value for missing highlight group.
+  return map_get(cstr_t, int)(highlight_unames, name_u);
 }
 
 /// Lookup a highlight group name and return its attributes.
@@ -7524,17 +7540,6 @@ char_u *syn_id2name(int id)
   return HL_TABLE()[id - 1].sg_name;
 }
 
-/*
- * Like syn_name2id(), but take a pointer + length argument.
- */
-int syn_namen2id(const char_u *linep, int len)
-{
-  char_u *name = vim_strnsave(linep, len);
-  int id = syn_name2id(name);
-  xfree(name);
-
-  return id;
-}
 
 /// Find highlight group name in the table and return its ID.
 /// If it doesn't exist yet, a new entry is created.
@@ -7543,14 +7548,11 @@ int syn_namen2id(const char_u *linep, int len)
 /// @param len length of \p pp
 ///
 /// @return 0 for failure else the id of the group
-int syn_check_group(const char_u *pp, int len)
+int syn_check_group(const char_u *name, int len)
 {
-  char_u  *name = vim_strnsave(pp, len);
-  int id = syn_name2id(name);
+  int id = syn_name2id_len(name, len);
   if (id == 0) {  // doesn't exist yet
-    id = syn_add_group(name);
-  } else {
-    xfree(name);
+    return syn_add_group(vim_strnsave(name, len));
   }
   return id;
 }
@@ -7593,7 +7595,7 @@ static int syn_add_group(char_u *name)
     return 0;
   }
 
-  char_u *const name_up = vim_strsave_up(name);
+  char *const name_up = (char *)vim_strsave_up(name);
 
   // Append another syntax_highlight entry.
   struct hl_group* hlgp = GA_APPEND_VIA_PTR(struct hl_group, &highlight_ga);
@@ -7605,7 +7607,11 @@ static int syn_add_group(char_u *name)
   hlgp->sg_blend = -1;
   hlgp->sg_name_u = name_up;
 
-  return highlight_ga.ga_len;               /* ID is index plus one */
+  int id = highlight_ga.ga_len;  // ID is index plus one
+
+  map_put(cstr_t, int)(highlight_unames, name_up, id);
+
+  return id;
 }
 
 /// When, just after calling syn_add_group(), an error is discovered, this
@@ -7613,8 +7619,10 @@ static int syn_add_group(char_u *name)
 static void syn_unadd_group(void)
 {
   highlight_ga.ga_len--;
-  xfree(HL_TABLE()[highlight_ga.ga_len].sg_name);
-  xfree(HL_TABLE()[highlight_ga.ga_len].sg_name_u);
+  HlGroup *item = &HL_TABLE()[highlight_ga.ga_len];
+  map_del(cstr_t, int)(highlight_unames, item->sg_name_u);
+  xfree(item->sg_name);
+  xfree(item->sg_name_u);
 }
 
 

src/nvim/testdir/test_match.vim

@@ -157,7 +157,10 @@ func Test_match_error()
 endfunc
 
 func Test_matchadd_error()
-  call assert_fails("call matchadd('GroupDoesNotExist', 'X')", 'E28:')
+  call clearmatches()
+  " Nvim: not an error anymore:
+  call matchadd('GroupDoesNotExist', 'X')
+  call assert_equal([{'group': 'GroupDoesNotExist', 'pattern': 'X', 'priority': 10, 'id': 13}], getmatches())
   call assert_fails("call matchadd('Search', '\\(')", 'E475:')
   call assert_fails("call matchadd('Search', 'XXX', 1, 123, 1)", 'E715:')
   call assert_fails("call matchadd('Error', 'XXX', 1, 3)", 'E798:')

src/nvim/window.c

@@ -6534,8 +6534,7 @@ int match_add(win_T *wp, const char *const grp, const char *const pat,
       cur = cur->next;
     }
   }
-  if ((hlg_id = syn_name2id((const char_u *)grp)) == 0) {
-    EMSG2(_(e_nogroup), grp);
+  if ((hlg_id = syn_check_group((const char_u *)grp, strlen(grp))) == 0) {
     return -1;
   }
   if (pat != NULL && (regprog = vim_regcomp((char_u *)pat, RE_MAGIC)) == NULL) {

test/functional/eval/match_functions_spec.lua

@@ -6,7 +6,6 @@ local clear = helpers.clear
 local funcs = helpers.funcs
 local command = helpers.command
 local exc_exec = helpers.exc_exec
-local pcall_err = helpers.pcall_err
 
 before_each(clear)
 
@@ -40,13 +39,13 @@ describe('setmatches()', function()
     }}, funcs.getmatches())
   end)
 
-  it('fails with -1 if highlight group is not defined', function()
-    eq('Vim:E28: No such highlight group name: 1',
-      pcall_err(funcs.setmatches, {{group=1, pattern=2, id=3, priority=4}}))
-    eq({}, funcs.getmatches())
-    eq('Vim:E28: No such highlight group name: 1',
-      pcall_err(funcs.setmatches, {{group=1, pos1={2}, pos2={6}, id=3, priority=4, conceal=5}}))
-    eq({}, funcs.getmatches())
+  it('does not fail if highlight group is not defined', function()
+    eq(0, funcs.setmatches{{group=1, pattern=2, id=3, priority=4}})
+    eq({{group='1', pattern='2', id=3, priority=4}},
+       funcs.getmatches())
+    eq(0, funcs.setmatches{{group=1, pos1={2}, pos2={6}, id=3, priority=4, conceal=5}})
+    eq({{group='1', pos1={2}, pos2={6}, id=3, priority=4, conceal='5'}},
+       funcs.getmatches())
   end)
 end)