refactor(highlight): make hlattrs2dict always use pre-allocated dict

hlattrs2dict used to work with both allocated and unallocated
dicts which was quite messy. Now always delegate allocation to caller.

5 files changed, 66 insertions, 68 deletions

diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index e34dcbdb46..110bd6920e 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -748,10 +748,12 @@ static void remote_ui_hl_attr_define(UI *ui, Integer id, HlAttrs rgb_attrs, HlAt
   UIData *data = ui->data;
   Array args = data->call_buf;
   ADD_C(args, INTEGER_OBJ(id));
-  MAXSIZE_TEMP_DICT(rgb, 16);
-  MAXSIZE_TEMP_DICT(cterm, 16);
-  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(&rgb, rgb_attrs, true)));
-  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(&cterm, cterm_attrs, false)));
+  MAXSIZE_TEMP_DICT(rgb, HLATTRS_DICT_SIZE);
+  MAXSIZE_TEMP_DICT(cterm, HLATTRS_DICT_SIZE);
+  hlattrs2dict(&rgb, rgb_attrs, true);
+  hlattrs2dict(&cterm, rgb_attrs, false);
+  ADD_C(args, DICTIONARY_OBJ(rgb));
+  ADD_C(args, DICTIONARY_OBJ(cterm));
 
   if (ui->ui_ext[kUIHlState]) {
     ADD_C(args, ARRAY_OBJ(info));
@@ -771,8 +773,9 @@ static void remote_ui_highlight_set(UI *ui, int id)
     return;
   }
   data->hl_id = id;
-  MAXSIZE_TEMP_DICT(dict, 16);
-  ADD_C(args, DICTIONARY_OBJ(hlattrs2dict(&dict, syn_attr2entry(id), ui->rgb)));
+  MAXSIZE_TEMP_DICT(dict, HLATTRS_DICT_SIZE);
+  hlattrs2dict(&dict, syn_attr2entry(id), ui->rgb);
+  ADD_C(args, DICTIONARY_OBJ(dict));
   push_call(ui, "highlight_set", args);
 }
 
@@ -952,65 +955,63 @@ static void remote_ui_flush(UI *ui)
   }
 }
 
-static Array translate_contents(UI *ui, Array contents)
+static Array translate_contents(UI *ui, Array contents, Arena *arena)
 {
-  Array new_contents = ARRAY_DICT_INIT;
+  Array new_contents = arena_array(arena, contents.size);
   for (size_t i = 0; i < contents.size; i++) {
     Array item = contents.items[i].data.array;
-    Array new_item = ARRAY_DICT_INIT;
+    Array new_item = arena_array(arena, 2);
     int attr = (int)item.items[0].data.integer;
     if (attr) {
-      Dictionary rgb_attrs = hlattrs2dict(NULL, syn_attr2entry(attr), ui->rgb);
+      Dictionary rgb_attrs = arena_dict(arena, HLATTRS_DICT_SIZE);
+      hlattrs2dict(&rgb_attrs, syn_attr2entry(attr), ui->rgb);
       ADD(new_item, DICTIONARY_OBJ(rgb_attrs));
     } else {
       ADD(new_item, DICTIONARY_OBJ((Dictionary)ARRAY_DICT_INIT));
     }
-    ADD(new_item, copy_object(item.items[1], NULL));
+    ADD(new_item, item.items[1]);
     ADD(new_contents, ARRAY_OBJ(new_item));
   }
   return new_contents;
 }
 
-static Array translate_firstarg(UI *ui, Array args)
+static Array translate_firstarg(UI *ui, Array args, Arena *arena)
 {
-  Array new_args = ARRAY_DICT_INIT;
+  Array new_args = arena_array(arena, args.size);
   Array contents = args.items[0].data.array;
 
-  ADD(new_args, ARRAY_OBJ(translate_contents(ui, contents)));
+  ADD_C(new_args, ARRAY_OBJ(translate_contents(ui, contents, arena)));
   for (size_t i = 1; i < args.size; i++) {
-    ADD(new_args, copy_object(args.items[i], NULL));
+    ADD(new_args, args.items[i]);
   }
   return new_args;
 }
 
 static void remote_ui_event(UI *ui, char *name, Array args)
 {
+  Arena arena = ARENA_EMPTY;
   UIData *data = ui->data;
   if (!ui->ui_ext[kUILinegrid]) {
     // the representation of highlights in cmdline changed, translate back
     // never consumes args
     if (strequal(name, "cmdline_show")) {
-      Array new_args = translate_firstarg(ui, args);
+      Array new_args = translate_firstarg(ui, args, &arena);
       push_call(ui, name, new_args);
-      api_free_array(new_args);
-      return;
+      goto free_ret;
     } else if (strequal(name, "cmdline_block_show")) {
       Array new_args = data->call_buf;
       Array block = args.items[0].data.array;
-      Array new_block = ARRAY_DICT_INIT;
+      Array new_block = arena_array(&arena, block.size);
       for (size_t i = 0; i < block.size; i++) {
-        ADD(new_block,
-            ARRAY_OBJ(translate_contents(ui, block.items[i].data.array)));
+        ADD_C(new_block, ARRAY_OBJ(translate_contents(ui, block.items[i].data.array, &arena)));
       }
       ADD_C(new_args, ARRAY_OBJ(new_block));
       push_call(ui, name, new_args);
-      api_free_array(new_block);
-      return;
+      goto free_ret;
     } else if (strequal(name, "cmdline_block_append")) {
-      Array new_args = translate_firstarg(ui, args);
+      Array new_args = translate_firstarg(ui, args, &arena);
       push_call(ui, name, new_args);
-      api_free_array(new_args);
-      return;
+      goto free_ret;
     }
   }
 
@@ -1022,19 +1023,18 @@ static void remote_ui_event(UI *ui, char *name, Array args)
       if (data->wildmenu_active) {
         Array new_args = data->call_buf;
         Array items = args.items[0].data.array;
-        Array new_items = ARRAY_DICT_INIT;
+        Array new_items = arena_array(&arena, items.size);
         for (size_t i = 0; i < items.size; i++) {
-          ADD(new_items, copy_object(items.items[i].data.array.items[0], NULL));
+          ADD_C(new_items, items.items[i].data.array.items[0]);
         }
         ADD_C(new_args, ARRAY_OBJ(new_items));
         push_call(ui, "wildmenu_show", new_args);
-        api_free_array(new_items);
         if (args.items[1].data.integer != -1) {
           Array new_args2 = data->call_buf;
           ADD_C(new_args2, args.items[1]);
           push_call(ui, "wildmenu_select", new_args2);
         }
-        return;
+        goto free_ret;
       }
     } else if (strequal(name, "popupmenu_select")) {
       if (data->wildmenu_active) {
@@ -1048,6 +1048,10 @@ static void remote_ui_event(UI *ui, char *name, Array args)
   }
 
   push_call(ui, name, args);
+  return;
+
+free_ret:
+  arena_mem_free(arena_finish(&arena));
 }
 
 static void remote_ui_inspect(UI *ui, Dictionary *info)
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 9e1f2dd631..83d6ba8dc7 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -79,7 +79,7 @@
 /// @param[out] err Error details, if any
 /// @return Highlight definition map
 /// @see nvim_get_hl_by_id
-Dictionary nvim_get_hl_by_name(String name, Boolean rgb, Error *err)
+Dictionary nvim_get_hl_by_name(String name, Boolean rgb, Arena *arena, Error *err)
   FUNC_API_SINCE(3)
 {
   Dictionary result = ARRAY_DICT_INIT;
@@ -89,8 +89,7 @@ Dictionary nvim_get_hl_by_name(String name, Boolean rgb, Error *err)
     api_set_error(err, kErrorTypeException, "Invalid highlight name: %s", name.data);
     return result;
   }
-  result = nvim_get_hl_by_id(id, rgb, err);
-  return result;
+  return nvim_get_hl_by_id(id, rgb, arena, err);
 }
 
 /// Gets a highlight definition by id. |hlID()|
@@ -99,7 +98,7 @@ Dictionary nvim_get_hl_by_name(String name, Boolean rgb, Error *err)
 /// @param[out] err Error details, if any
 /// @return Highlight definition map
 /// @see nvim_get_hl_by_name
-Dictionary nvim_get_hl_by_id(Integer hl_id, Boolean rgb, Error *err)
+Dictionary nvim_get_hl_by_id(Integer hl_id, Boolean rgb, Arena *arena, Error *err)
   FUNC_API_SINCE(3)
 {
   Dictionary dic = ARRAY_DICT_INIT;
@@ -108,7 +107,7 @@ Dictionary nvim_get_hl_by_id(Integer hl_id, Boolean rgb, Error *err)
     return dic;
   }
   int attrcode = syn_id2attr((int)hl_id);
-  return hl_get_attr_by_id(attrcode, rgb, err);
+  return hl_get_attr_by_id(attrcode, rgb, arena, err);
 }
 
 /// Gets a highlight group by name
@@ -120,10 +119,10 @@ Integer nvim_get_hl_id_by_name(String name)
   return syn_check_group(name.data, name.size);
 }
 
-Dictionary nvim__get_hl_defs(Integer ns_id, Error *err)
+Dictionary nvim__get_hl_defs(Integer ns_id, Arena *arena, Error *err)
 {
   if (ns_id == 0) {
-    return get_global_hl_defs();
+    return get_global_hl_defs(arena);
   }
   abort();
 }
@@ -1934,7 +1933,7 @@ void nvim_select_popupmenu_item(Integer item, Boolean insert, Boolean finish, Di
 }
 
 /// NB: if your UI doesn't use hlstate, this will not return hlstate first time
-Array nvim__inspect_cell(Integer grid, Integer row, Integer col, Error *err)
+Array nvim__inspect_cell(Integer grid, Integer row, Integer col, Arena *arena, Error *err)
 {
   Array ret = ARRAY_DICT_INIT;
 
@@ -1958,13 +1957,14 @@ Array nvim__inspect_cell(Integer grid, Integer row, Integer col, Error *err)
       || col < 0 || col >= g->cols) {
     return ret;
   }
+  ret = arena_array(arena, 3);
   size_t off = g->line_offset[(size_t)row] + (size_t)col;
-  ADD(ret, STRING_OBJ(cstr_to_string((char *)g->chars[off])));
+  ADD_C(ret, STRING_OBJ(cstr_as_string((char *)g->chars[off])));
   int attr = g->attrs[off];
-  ADD(ret, DICTIONARY_OBJ(hl_get_attr_by_id(attr, true, err)));
+  ADD_C(ret, DICTIONARY_OBJ(hl_get_attr_by_id(attr, true, arena, err)));
   // will not work first time
   if (!highlight_use_hlstate()) {
-    ADD(ret, ARRAY_OBJ(hl_inspect(attr)));
+    ADD_C(ret, ARRAY_OBJ(hl_inspect(attr)));
   }
   return ret;
 }
diff --git a/src/nvim/highlight.c b/src/nvim/highlight.c
index a78b933108..e42cb66422 100644
--- a/src/nvim/highlight.c
+++ b/src/nvim/highlight.c
@@ -788,7 +788,7 @@ HlAttrs syn_attr2entry(int attr)
 }
 
 /// Gets highlight description for id `attr_id` as a map.
-Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Error *err)
+Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Arena *arena, Error *err)
 {
   Dictionary dic = ARRAY_DICT_INIT;
 
@@ -801,25 +801,21 @@ Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Error *err)
                   "Invalid attribute id: %" PRId64, attr_id);
     return dic;
   }
-
-  return hlattrs2dict(NULL, syn_attr2entry((int)attr_id), rgb);
+  Dictionary retval = arena_dict(arena, HLATTRS_DICT_SIZE);
+  hlattrs2dict(&retval, syn_attr2entry((int)attr_id), rgb);
+  return retval;
 }
 
 /// Converts an HlAttrs into Dictionary
 ///
-/// @param[out] hl optional pre-allocated dictionary for return value
-///                if present, must be allocated with at least 16 elements!
+/// @param[in/out] hl Dictionary with pre-allocated space for HLATTRS_DICT_SIZE elements
 /// @param[in] aep data to convert
 /// @param use_rgb use 'gui*' settings if true, else resorts to 'cterm*'
-Dictionary hlattrs2dict(Dictionary *hl_alloc, HlAttrs ae, bool use_rgb)
+void hlattrs2dict(Dictionary *dict, HlAttrs ae, bool use_rgb)
 {
+  assert(dict->capacity >= HLATTRS_DICT_SIZE);  // at most 16 items
+  Dictionary hl = *dict;
   int mask  = use_rgb ? ae.rgb_ae_attr : ae.cterm_ae_attr;
-  Dictionary hl = ARRAY_DICT_INIT;
-  if (hl_alloc) {
-    hl = *hl_alloc;
-  } else {
-    kv_ensure_space(hl, 16);
-  }
 
   if (mask & HL_BOLD) {
     PUT_C(hl, "bold", BOOLEAN_OBJ(true));
@@ -899,14 +895,7 @@ Dictionary hlattrs2dict(Dictionary *hl_alloc, HlAttrs ae, bool use_rgb)
     PUT_C(hl, "blend", INTEGER_OBJ(ae.hl_blend));
   }
 
-  if (hl_alloc) {
-    *hl_alloc = hl;
-    return hl;
-  } else {
-    Dictionary allocated = copy_dictionary(hl, NULL);
-    kv_destroy(hl);
-    return allocated;
-  }
+  *dict = hl;
 }
 
 HlAttrs dict2hlattrs(Dict(highlight) *dict, bool use_rgb, int *link_id, Error *err)
@@ -1086,6 +1075,7 @@ int object_to_color(Object val, char *key, bool rgb, Error *err)
 
 Array hl_inspect(int attr)
 {
+  // TODO(bfredl): use arena allocation
   Array ret = ARRAY_DICT_INIT;
   if (hlstate_active) {
     hl_inspect_impl(&ret, attr);
diff --git a/src/nvim/highlight.h b/src/nvim/highlight.h
index 50299bb91c..e85e3859e2 100644
--- a/src/nvim/highlight.h
+++ b/src/nvim/highlight.h
@@ -19,6 +19,8 @@ static inline int win_hl_attr(win_T *wp, int hlf)
   return ((wp->w_ns_hl_attr && ns_hl_fast < 0) ? wp->w_ns_hl_attr : hl_attr_active)[hlf];
 }
 
+#define HLATTRS_DICT_SIZE 16
+
 #define HL_SET_DEFAULT_COLORS(rgb_fg, rgb_bg, rgb_sp) \
   do { \
     bool dark_ = (*p_bg == 'd'); \
diff --git a/src/nvim/highlight_group.c b/src/nvim/highlight_group.c
index ed1f0185b7..97d4b421fe 100644
--- a/src/nvim/highlight_group.c
+++ b/src/nvim/highlight_group.c
@@ -1466,19 +1466,21 @@ static void highlight_list_one(const int id)
   }
 }
 
-Dictionary get_global_hl_defs(void)
+Dictionary get_global_hl_defs(Arena *arena)
 {
-  Dictionary rv = ARRAY_DICT_INIT;
-  for (int i = 1; i <= highlight_ga.ga_len && !got_int; i++) {
+  Dictionary rv = arena_dict(arena, (size_t)highlight_ga.ga_len);
+  for (int i = 1; i <= highlight_ga.ga_len; i++) {
     Dictionary attrs = ARRAY_DICT_INIT;
     HlGroup *h = &hl_table[i - 1];
     if (h->sg_attr > 0) {
-      attrs = hlattrs2dict(NULL, syn_attr2entry(h->sg_attr), true);
+      attrs = arena_dict(arena, HLATTRS_DICT_SIZE);
+      hlattrs2dict(&attrs, syn_attr2entry(h->sg_attr), true);
     } else if (h->sg_link > 0) {
-      const char *link = (const char *)hl_table[h->sg_link - 1].sg_name;
-      PUT(attrs, "link", STRING_OBJ(cstr_to_string(link)));
+      attrs = arena_dict(arena, 1);
+      char *link = (char *)hl_table[h->sg_link - 1].sg_name;
+      PUT_C(attrs, "link", STRING_OBJ(cstr_as_string(link)));
     }
-    PUT(rv, (const char *)h->sg_name, DICTIONARY_OBJ(attrs));
+    PUT_C(rv, (char *)h->sg_name, DICTIONARY_OBJ(attrs));
   }
 
   return rv;