refactor(api): use arena for metadata; msgpack_rpc_to_object delenda est

Note: kSDItemHeader is something is _written_ by nvim in the shada file
to identify it for debugging purposes outside of nvim. But this data wasn't ever used by
neovim after reading the file back, So I removed the parsing of it for now.

6 files changed, 71 insertions, 312 deletions

diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index 983f1a4fed..63c5689109 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -30,6 +30,7 @@
 #include "nvim/memory_defs.h"
 #include "nvim/message.h"
 #include "nvim/msgpack_rpc/helpers.h"
+#include "nvim/msgpack_rpc/unpacker.h"
 #include "nvim/pos_defs.h"
 #include "nvim/types_defs.h"
 #include "nvim/ui.h"
@@ -650,102 +651,87 @@ void api_clear_error(Error *value)
   value->type = kErrorTypeNone;
 }
 
+// initialized once, never freed
+static ArenaMem mem_for_metadata = NULL;
+
 /// @returns a shared value. caller must not modify it!
 Dictionary api_metadata(void)
 {
   static Dictionary metadata = ARRAY_DICT_INIT;
 
   if (!metadata.size) {
-    PUT(metadata, "version", DICTIONARY_OBJ(version_dict()));
-    init_function_metadata(&metadata);
-    init_ui_event_metadata(&metadata);
-    init_error_type_metadata(&metadata);
-    init_type_metadata(&metadata);
+    Arena arena = ARENA_EMPTY;
+    Error err = ERROR_INIT;
+    metadata = arena_dict(&arena, 6);
+    PUT_C(metadata, "version", DICTIONARY_OBJ(version_dict(&arena)));
+    PUT_C(metadata, "functions",
+          unpack((char *)funcs_metadata, sizeof(funcs_metadata), &arena, &err));
+    if (ERROR_SET(&err)) {
+      abort();
+    }
+    PUT_C(metadata, "ui_events",
+          unpack((char *)ui_events_metadata, sizeof(ui_events_metadata), &arena, &err));
+    if (ERROR_SET(&err)) {
+      abort();
+    }
+    PUT_C(metadata, "ui_options", ARRAY_OBJ(ui_options_metadata(&arena)));
+    PUT_C(metadata, "error_types", DICTIONARY_OBJ(error_type_metadata(&arena)));
+    PUT_C(metadata, "types", DICTIONARY_OBJ(type_metadata(&arena)));
+    mem_for_metadata = arena_finish(&arena);
   }
 
   return metadata;
 }
 
-static void init_function_metadata(Dictionary *metadata)
+static Array ui_options_metadata(Arena *arena)
 {
-  msgpack_unpacked unpacked;
-  msgpack_unpacked_init(&unpacked);
-  if (msgpack_unpack_next(&unpacked,
-                          (const char *)funcs_metadata,
-                          sizeof(funcs_metadata),
-                          NULL) != MSGPACK_UNPACK_SUCCESS) {
-    abort();
-  }
-  Object functions;
-  msgpack_rpc_to_object(&unpacked.data, &functions);
-  msgpack_unpacked_destroy(&unpacked);
-  PUT(*metadata, "functions", functions);
-}
-
-static void init_ui_event_metadata(Dictionary *metadata)
-{
-  msgpack_unpacked unpacked;
-  msgpack_unpacked_init(&unpacked);
-  if (msgpack_unpack_next(&unpacked,
-                          (const char *)ui_events_metadata,
-                          sizeof(ui_events_metadata),
-                          NULL) != MSGPACK_UNPACK_SUCCESS) {
-    abort();
-  }
-  Object ui_events;
-  msgpack_rpc_to_object(&unpacked.data, &ui_events);
-  msgpack_unpacked_destroy(&unpacked);
-  PUT(*metadata, "ui_events", ui_events);
-  Array ui_options = ARRAY_DICT_INIT;
-  ADD(ui_options, CSTR_TO_OBJ("rgb"));
+  Array ui_options = arena_array(arena, kUIExtCount + 1);
+  ADD_C(ui_options, CSTR_AS_OBJ("rgb"));
   for (UIExtension i = 0; i < kUIExtCount; i++) {
     if (ui_ext_names[i][0] != '_') {
-      ADD(ui_options, CSTR_TO_OBJ(ui_ext_names[i]));
+      ADD_C(ui_options, CSTR_AS_OBJ(ui_ext_names[i]));
     }
   }
-  PUT(*metadata, "ui_options", ARRAY_OBJ(ui_options));
+  return ui_options;
 }
 
-static void init_error_type_metadata(Dictionary *metadata)
+static Dictionary error_type_metadata(Arena *arena)
 {
-  Dictionary types = ARRAY_DICT_INIT;
+  Dictionary types = arena_dict(arena, 2);
 
-  Dictionary exception_metadata = ARRAY_DICT_INIT;
-  PUT(exception_metadata, "id", INTEGER_OBJ(kErrorTypeException));
+  Dictionary exception_metadata = arena_dict(arena, 1);
+  PUT_C(exception_metadata, "id", INTEGER_OBJ(kErrorTypeException));
 
-  Dictionary validation_metadata = ARRAY_DICT_INIT;
-  PUT(validation_metadata, "id", INTEGER_OBJ(kErrorTypeValidation));
+  Dictionary validation_metadata = arena_dict(arena, 1);
+  PUT_C(validation_metadata, "id", INTEGER_OBJ(kErrorTypeValidation));
 
-  PUT(types, "Exception", DICTIONARY_OBJ(exception_metadata));
-  PUT(types, "Validation", DICTIONARY_OBJ(validation_metadata));
+  PUT_C(types, "Exception", DICTIONARY_OBJ(exception_metadata));
+  PUT_C(types, "Validation", DICTIONARY_OBJ(validation_metadata));
 
-  PUT(*metadata, "error_types", DICTIONARY_OBJ(types));
+  return types;
 }
 
-static void init_type_metadata(Dictionary *metadata)
+static Dictionary type_metadata(Arena *arena)
 {
-  Dictionary types = ARRAY_DICT_INIT;
+  Dictionary types = arena_dict(arena, 3);
 
-  Dictionary buffer_metadata = ARRAY_DICT_INIT;
-  PUT(buffer_metadata, "id",
-      INTEGER_OBJ(kObjectTypeBuffer - EXT_OBJECT_TYPE_SHIFT));
-  PUT(buffer_metadata, "prefix", CSTR_TO_OBJ("nvim_buf_"));
+  Dictionary buffer_metadata = arena_dict(arena, 2);
+  PUT_C(buffer_metadata, "id", INTEGER_OBJ(kObjectTypeBuffer - EXT_OBJECT_TYPE_SHIFT));
+  PUT_C(buffer_metadata, "prefix", CSTR_AS_OBJ("nvim_buf_"));
 
-  Dictionary window_metadata = ARRAY_DICT_INIT;
-  PUT(window_metadata, "id",
-      INTEGER_OBJ(kObjectTypeWindow - EXT_OBJECT_TYPE_SHIFT));
-  PUT(window_metadata, "prefix", CSTR_TO_OBJ("nvim_win_"));
+  Dictionary window_metadata = arena_dict(arena, 2);
+  PUT_C(window_metadata, "id", INTEGER_OBJ(kObjectTypeWindow - EXT_OBJECT_TYPE_SHIFT));
+  PUT_C(window_metadata, "prefix", CSTR_AS_OBJ("nvim_win_"));
 
-  Dictionary tabpage_metadata = ARRAY_DICT_INIT;
-  PUT(tabpage_metadata, "id",
-      INTEGER_OBJ(kObjectTypeTabpage - EXT_OBJECT_TYPE_SHIFT));
-  PUT(tabpage_metadata, "prefix", CSTR_TO_OBJ("nvim_tabpage_"));
+  Dictionary tabpage_metadata = arena_dict(arena, 2);
+  PUT_C(tabpage_metadata, "id", INTEGER_OBJ(kObjectTypeTabpage - EXT_OBJECT_TYPE_SHIFT));
+  PUT_C(tabpage_metadata, "prefix", CSTR_AS_OBJ("nvim_tabpage_"));
 
-  PUT(types, "Buffer", DICTIONARY_OBJ(buffer_metadata));
-  PUT(types, "Window", DICTIONARY_OBJ(window_metadata));
-  PUT(types, "Tabpage", DICTIONARY_OBJ(tabpage_metadata));
+  PUT_C(types, "Buffer", DICTIONARY_OBJ(buffer_metadata));
+  PUT_C(types, "Window", DICTIONARY_OBJ(window_metadata));
+  PUT_C(types, "Tabpage", DICTIONARY_OBJ(tabpage_metadata));
 
-  PUT(*metadata, "types", DICTIONARY_OBJ(types));
+  return types;
 }
 
 // all the copy_[object] functions allow arena=NULL,
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 7a19ccf295..d161dca050 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -2022,10 +2022,10 @@ void nvim__invalidate_glyph_cache(void)
   must_redraw = UPD_CLEAR;
 }
 
-Object nvim__unpack(String str, Error *err)
+Object nvim__unpack(String str, Arena *arena, Error *err)
   FUNC_API_FAST
 {
-  return unpack(str.data, str.size, err);
+  return unpack(str.data, str.size, arena, err);
 }
 
 /// Deletes an uppercase/file named mark. See |mark-motions|.
diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c
index 6caf50d319..c0c519848c 100644
--- a/src/nvim/msgpack_rpc/helpers.c
+++ b/src/nvim/msgpack_rpc/helpers.c
@@ -36,237 +36,8 @@ void msgpack_rpc_helpers_free_all_mem(void)
 }
 #endif
 
-typedef struct {
-  const msgpack_object *mobj;
-  Object *aobj;
-  bool container;
-  size_t idx;
-} MPToAPIObjectStackItem;
-
 // uncrustify:off
 
-/// Convert type used by msgpack parser to Nvim API type.
-///
-/// @param[in]  obj  Msgpack value to convert.
-/// @param[out]  arg  Location where result of conversion will be saved.
-///
-/// @return true in case of success, false otherwise.
-bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg)
-  FUNC_ATTR_NONNULL_ALL
-{
-  bool ret = true;
-  kvec_withinit_t(MPToAPIObjectStackItem, 2) stack = KV_INITIAL_VALUE;
-  kvi_init(stack);
-  kvi_push(stack, ((MPToAPIObjectStackItem) {
-    .mobj = obj,
-    .aobj = arg,
-    .container = false,
-    .idx = 0,
-  }));
-  while (ret && kv_size(stack)) {
-    MPToAPIObjectStackItem cur = kv_last(stack);
-    if (!cur.container) {
-      *cur.aobj = NIL;
-    }
-    switch (cur.mobj->type) {
-    case MSGPACK_OBJECT_NIL:
-      break;
-    case MSGPACK_OBJECT_BOOLEAN:
-      *cur.aobj = BOOLEAN_OBJ(cur.mobj->via.boolean);
-      break;
-    case MSGPACK_OBJECT_NEGATIVE_INTEGER:
-      STATIC_ASSERT(sizeof(Integer) == sizeof(cur.mobj->via.i64),
-                    "Msgpack integer size does not match API integer");
-      *cur.aobj = INTEGER_OBJ(cur.mobj->via.i64);
-      break;
-    case MSGPACK_OBJECT_POSITIVE_INTEGER:
-      STATIC_ASSERT(sizeof(Integer) == sizeof(cur.mobj->via.u64),
-                    "Msgpack integer size does not match API integer");
-      if (cur.mobj->via.u64 > API_INTEGER_MAX) {
-        ret = false;
-      } else {
-        *cur.aobj = INTEGER_OBJ((Integer)cur.mobj->via.u64);
-      }
-      break;
-    case MSGPACK_OBJECT_FLOAT32:
-    case MSGPACK_OBJECT_FLOAT64:
-    {
-      STATIC_ASSERT(sizeof(Float) == sizeof(cur.mobj->via.f64),
-                    "Msgpack floating-point size does not match API integer");
-      *cur.aobj = FLOAT_OBJ(cur.mobj->via.f64);
-      break;
-    }
-#define STR_CASE(type, attr, obj, dest, conv) \
-  case type: { \
-      dest = conv(((String) { \
-      .size = obj->via.attr.size, \
-      .data = (obj->via.attr.ptr == NULL || obj->via.attr.size == 0 \
-               ? xmemdupz("", 0) \
-               : xmemdupz(obj->via.attr.ptr, obj->via.attr.size)), })); \
-      break; \
-  }
-      STR_CASE(MSGPACK_OBJECT_STR, str, cur.mobj, *cur.aobj, STRING_OBJ)
-      STR_CASE(MSGPACK_OBJECT_BIN, bin, cur.mobj, *cur.aobj, STRING_OBJ)
-    case MSGPACK_OBJECT_ARRAY: {
-      const size_t size = cur.mobj->via.array.size;
-      if (cur.container) {
-        if (cur.idx >= size) {
-          (void)kv_pop(stack);
-        } else {
-          const size_t idx = cur.idx;
-          cur.idx++;
-          kv_last(stack) = cur;
-          kvi_push(stack, ((MPToAPIObjectStackItem) {
-              .mobj = &cur.mobj->via.array.ptr[idx],
-              .aobj = &cur.aobj->data.array.items[idx],
-              .container = false,
-          }));
-        }
-      } else {
-        *cur.aobj = ARRAY_OBJ(((Array) {
-            .size = size,
-            .capacity = size,
-            .items = (size > 0
-                      ? xcalloc(size, sizeof(*cur.aobj->data.array.items))
-                      : NULL),
-        }));
-        cur.container = true;
-        kv_last(stack) = cur;
-      }
-      break;
-    }
-    case MSGPACK_OBJECT_MAP: {
-      const size_t size = cur.mobj->via.map.size;
-      if (cur.container) {
-        if (cur.idx >= size) {
-          (void)kv_pop(stack);
-        } else {
-          const size_t idx = cur.idx;
-          cur.idx++;
-          kv_last(stack) = cur;
-          const msgpack_object *const key = &cur.mobj->via.map.ptr[idx].key;
-          switch (key->type) {
-#define ID(x) x
-          STR_CASE(MSGPACK_OBJECT_STR, str, key,
-                   cur.aobj->data.dictionary.items[idx].key, ID)
-          STR_CASE(MSGPACK_OBJECT_BIN, bin, key,
-                   cur.aobj->data.dictionary.items[idx].key, ID)
-#undef ID
-          case MSGPACK_OBJECT_NIL:
-          case MSGPACK_OBJECT_BOOLEAN:
-          case MSGPACK_OBJECT_POSITIVE_INTEGER:
-          case MSGPACK_OBJECT_NEGATIVE_INTEGER:
-          case MSGPACK_OBJECT_FLOAT32:
-          case MSGPACK_OBJECT_FLOAT64:
-          case MSGPACK_OBJECT_EXT:
-          case MSGPACK_OBJECT_MAP:
-          case MSGPACK_OBJECT_ARRAY:
-            ret = false;
-            break;
-          }
-          if (ret) {
-            kvi_push(stack, ((MPToAPIObjectStackItem) {
-                .mobj = &cur.mobj->via.map.ptr[idx].val,
-                .aobj = &cur.aobj->data.dictionary.items[idx].value,
-                .container = false,
-            }));
-          }
-        }
-      } else {
-        *cur.aobj = DICTIONARY_OBJ(((Dictionary) {
-            .size = size,
-            .capacity = size,
-            .items = (size > 0
-                      ? xcalloc(size, sizeof(*cur.aobj->data.dictionary.items))
-                      : NULL),
-        }));
-        cur.container = true;
-        kv_last(stack) = cur;
-      }
-      break;
-    }
-    case MSGPACK_OBJECT_EXT:
-      if (0 <= cur.mobj->via.ext.type && cur.mobj->via.ext.type <= EXT_OBJECT_TYPE_MAX) {
-        cur.aobj->type = (ObjectType)(cur.mobj->via.ext.type + EXT_OBJECT_TYPE_SHIFT);
-        msgpack_object data;
-        msgpack_unpack_return status = msgpack_unpack(cur.mobj->via.ext.ptr, cur.mobj->via.ext.size,
-                                                      NULL, &zone, &data);
-
-        if (status != MSGPACK_UNPACK_SUCCESS || data.type != MSGPACK_OBJECT_POSITIVE_INTEGER) {
-          ret = false;
-          break;
-        }
-        cur.aobj->data.integer = (handle_T)data.via.i64;
-        ret = true;
-      }
-      break;
-#undef STR_CASE
-    }
-    if (!cur.container) {
-      (void)kv_pop(stack);
-    }
-  }
-  kvi_destroy(stack);
-  return ret;
-}
-
-static bool msgpack_rpc_to_string(const msgpack_object *const obj, String *const arg)
-  FUNC_ATTR_NONNULL_ALL
-{
-  if (obj->type == MSGPACK_OBJECT_BIN || obj->type == MSGPACK_OBJECT_STR) {
-    arg->data = obj->via.bin.ptr != NULL
-                    ? xmemdupz(obj->via.bin.ptr, obj->via.bin.size)
-                    : NULL;
-    arg->size = obj->via.bin.size;
-    return true;
-  }
-  return false;
-}
-
-bool msgpack_rpc_to_array(const msgpack_object *const obj, Array *const arg)
-  FUNC_ATTR_NONNULL_ALL
-{
-  if (obj->type != MSGPACK_OBJECT_ARRAY) {
-    return false;
-  }
-
-  arg->size = obj->via.array.size;
-  arg->items = xcalloc(obj->via.array.size, sizeof(Object));
-
-  for (uint32_t i = 0; i < obj->via.array.size; i++) {
-    if (!msgpack_rpc_to_object(obj->via.array.ptr + i, &arg->items[i])) {
-      return false;
-    }
-  }
-
-  return true;
-}
-
-bool msgpack_rpc_to_dictionary(const msgpack_object *const obj, Dictionary *const arg)
-  FUNC_ATTR_NONNULL_ALL
-{
-  if (obj->type != MSGPACK_OBJECT_MAP) {
-    return false;
-  }
-
-  arg->size = obj->via.array.size;
-  arg->items = xcalloc(obj->via.map.size, sizeof(KeyValuePair));
-
-  for (uint32_t i = 0; i < obj->via.map.size; i++) {
-    if (!msgpack_rpc_to_string(&obj->via.map.ptr[i].key,
-                               &arg->items[i].key)) {
-      return false;
-    }
-
-    if (!msgpack_rpc_to_object(&obj->via.map.ptr[i].val,
-                               &arg->items[i].value)) {
-      return false;
-    }
-  }
-
-  return true;
-}
-
 void msgpack_rpc_from_boolean(Boolean result, msgpack_packer *res)
   FUNC_ATTR_NONNULL_ARG(2)
 {
diff --git a/src/nvim/msgpack_rpc/unpacker.c b/src/nvim/msgpack_rpc/unpacker.c
index 38263381bf..6bc138f65a 100644
--- a/src/nvim/msgpack_rpc/unpacker.c
+++ b/src/nvim/msgpack_rpc/unpacker.c
@@ -18,15 +18,18 @@
 # include "msgpack_rpc/unpacker.c.generated.h"
 #endif
 
-Object unpack(const char *data, size_t size, Error *err)
+Object unpack(const char *data, size_t size, Arena *arena, Error *err)
 {
   Unpacker unpacker;
   mpack_parser_init(&unpacker.parser, 0);
   unpacker.parser.data.p = &unpacker;
+  unpacker.arena = *arena;
 
   int result = mpack_parse(&unpacker.parser, &data, &size,
                            api_parse_enter, api_parse_exit);
 
+  *arena = unpacker.arena;
+
   if (result == MPACK_NOMEM) {
     api_set_error(err, kErrorTypeException, "object was too deep to unpack");
   } else if (result == MPACK_EOF) {
diff --git a/src/nvim/shada.c b/src/nvim/shada.c
index 2c8685adc7..f4a729d13d 100644
--- a/src/nvim/shada.c
+++ b/src/nvim/shada.c
@@ -3629,10 +3629,9 @@ shada_read_next_item_start:
   entry->data = sd_default_values[type_u64].data;
   switch ((ShadaEntryType)type_u64) {
   case kSDItemHeader:
-    if (!msgpack_rpc_to_dictionary(&(unpacked.data), &(entry->data.header))) {
-      semsg(_(READERR("header", "is not a dictionary")), initial_fpos);
-      goto shada_read_next_item_error;
-    }
+    // TODO(bfredl): header is written to file and provides useful debugging
+    // info. It is never read by nvim (earlier we parsed it back to a
+    // Dictionary, but that value was never used)
     break;
   case kSDItemSearchPattern: {
     if (unpacked.data.type != MSGPACK_OBJECT_MAP) {
diff --git a/src/nvim/version.c b/src/nvim/version.c
index 8568f67aa1..a3f825c324 100644
--- a/src/nvim/version.c
+++ b/src/nvim/version.c
@@ -2550,21 +2550,21 @@ bool has_vim_patch(int n)
   return false;
 }
 
-Dictionary version_dict(void)
+Dictionary version_dict(Arena *arena)
 {
-  Dictionary d = ARRAY_DICT_INIT;
-  PUT(d, "major", INTEGER_OBJ(NVIM_VERSION_MAJOR));
-  PUT(d, "minor", INTEGER_OBJ(NVIM_VERSION_MINOR));
-  PUT(d, "patch", INTEGER_OBJ(NVIM_VERSION_PATCH));
+  Dictionary d = arena_dict(arena, 8);
+  PUT_C(d, "major", INTEGER_OBJ(NVIM_VERSION_MAJOR));
+  PUT_C(d, "minor", INTEGER_OBJ(NVIM_VERSION_MINOR));
+  PUT_C(d, "patch", INTEGER_OBJ(NVIM_VERSION_PATCH));
 #ifndef NVIM_VERSION_BUILD
-  PUT(d, "build", NIL);
+  PUT_C(d, "build", NIL);
 #else
-  PUT(d, "build", CSTR_AS_OBJ(NVIM_VERSION_BUILD));
+  PUT_C(d, "build", STATIC_CSTR_AS_OBJ(NVIM_VERSION_BUILD));
 #endif
-  PUT(d, "prerelease", BOOLEAN_OBJ(NVIM_VERSION_PRERELEASE[0] != '\0'));
-  PUT(d, "api_level", INTEGER_OBJ(NVIM_API_LEVEL));
-  PUT(d, "api_compatible", INTEGER_OBJ(NVIM_API_LEVEL_COMPAT));
-  PUT(d, "api_prerelease", BOOLEAN_OBJ(NVIM_API_PRERELEASE));
+  PUT_C(d, "prerelease", BOOLEAN_OBJ(NVIM_VERSION_PRERELEASE[0] != '\0'));
+  PUT_C(d, "api_level", INTEGER_OBJ(NVIM_API_LEVEL));
+  PUT_C(d, "api_compatible", INTEGER_OBJ(NVIM_API_LEVEL_COMPAT));
+  PUT_C(d, "api_prerelease", BOOLEAN_OBJ(NVIM_API_PRERELEASE));
   return d;
 }