eval: Use better error messages when failing to dump values

Examples:

    let g:SR = [[]]
    call add(g:SR[0], g:SR)
    wshada
    " E952: Unable to dump variable g:SR: container references itself in index 0, index 0

    let g:F = {'_TYPE': v:msgpack_types.map, '_VAL': [[{'abc': 1}, function("tr")]]}
    wshada
    " E951: Error while dumping variable g:F, key {'abc': 1} at index 0 from special map, key '': attempt to dump function reference
    " (no msgpack#string available)
    " E951: Error while dumping variable g:F, key {="abc": 1} at index 0 from special map, key '': attempt to dump function reference
    " (msgpack#string available)

    let g:F = {'_TYPE': v:msgpack_types.map, '_VAL': [[g:SR, function("tr")]]}
    wshada
    " E951: Error while dumping variable g:F, key [[[[{E724@0}]]]] at index 0 from special map, index 1: attempt to dump function reference

    call msgpackdump([g:SR])
    " E952: Unable to dump msgpackdump() argument, index 0: container references itself in index 0, index 0

Not tested yet.

src/nvim/eval.c

@@ -6479,7 +6479,8 @@ failret:
 static int name##_convert_one_value(firstargtype firstargname, \
                                     MPConvStack *const mpstack, \
                                     typval_T *const tv, \
-                                    const int copyID) \
+                                    const int copyID, \
+                                    const char *const objname) \
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT \
 { \
   switch (tv->v_type) { \
@@ -6713,14 +6714,16 @@ name##_convert_one_value_regular_dict: \
   return OK; \
 } \
 \
-scope int vim_to_##name(firstargtype firstargname, typval_T *const tv) \
+scope int vim_to_##name(firstargtype firstargname, typval_T *const tv, \
+                        const char *const objname) \
   FUNC_ATTR_WARN_UNUSED_RESULT \
 { \
   current_copyID += COPYID_INC; \
   const int copyID = current_copyID; \
   MPConvStack mpstack; \
   kv_init(mpstack); \
-  if (name##_convert_one_value(firstargname, &mpstack, tv, copyID) == FAIL) { \
+  if (name##_convert_one_value(firstargname, &mpstack, tv, copyID, objname) \
+      == FAIL) { \
     goto vim_to_msgpack_error_ret; \
   } \
   while (kv_size(mpstack)) { \
@@ -6769,8 +6772,8 @@ scope int vim_to_##name(firstargtype firstargname, typval_T *const tv) \
         } \
         const list_T *const kv_pair = cur_mpsv->data.l.li->li_tv.vval.v_list; \
         if (name##_convert_one_value(firstargname, &mpstack, \
-                                     &kv_pair->lv_first->li_tv, copyID) \
+                                     &kv_pair->lv_first->li_tv, copyID, \
-            == FAIL) { \
+                                     objname) == FAIL) { \
           goto vim_to_msgpack_error_ret; \
         } \
         cur_tv = &kv_pair->lv_last->li_tv; \
@@ -6778,8 +6781,8 @@ scope int vim_to_##name(firstargtype firstargname, typval_T *const tv) \
         break; \
       } \
     } \
-    if (name##_convert_one_value(firstargname, &mpstack, cur_tv, copyID) \
+    if (name##_convert_one_value(firstargname, &mpstack, cur_tv, copyID, \
-        == FAIL) { \
+                                 objname) == FAIL) { \
       goto vim_to_msgpack_error_ret; \
     } \
   } \
@@ -6975,7 +6978,7 @@ static char *tv2string(typval_T *tv, size_t *len)
 {
   garray_T ga;
   ga_init(&ga, (int)sizeof(char), 80);
-  vim_to_string(&ga, tv);
+  vim_to_string(&ga, tv, "tv2string() argument");
   did_echo_string_emsg = false;
   if (len != NULL) {
     *len = (size_t) ga.ga_len;
@@ -7001,7 +7004,7 @@ static char *echo_string(typval_T *tv, size_t *len)
       ga_concat(&ga, tv->vval.v_string);
     }
   } else {
-    vim_to_echo(&ga, tv);
+    vim_to_echo(&ga, tv, ":echo argument");
     did_echo_string_emsg = false;
   }
   if (len != NULL) {
@@ -12686,6 +12689,100 @@ static inline bool vim_list_to_buf(const list_T *const list,
   return true;
 }
 
+/// Show a error message when converting to msgpack value
+///
+/// @param[in]  msg  Error message to dump. Must contain exactly two %s that
+///                  will be replaced with what was being dumped: first with
+///                  something like “F” or “function argument”, second with path
+///                  to the failed value.
+/// @param[in]  mpstack  Path to the failed value.
+/// @param[in]  objname  Dumped object name.
+///
+/// @return FAIL.
+static int conv_error(const char *const msg, const MPConvStack *const mpstack,
+                      const char *const objname)
+  FUNC_ATTR_NONNULL_ALL
+{
+  garray_T msg_ga;
+  ga_init(&msg_ga, (int)sizeof(char), 80);
+  char *const key_msg = _("key %s");
+  char *const key_pair_msg = _("key %s at index %i from special map");
+  char *const idx_msg = _("index %i");
+  for (size_t i = 0; i < kv_size(*mpstack); i++) {
+    if (i != 0) {
+      ga_concat(&msg_ga, (char_u *) ", ");
+    }
+    MPConvStackVal v = kv_A(*mpstack, i);
+    switch (v.type) {
+      case kMPConvDict: {
+        typval_T key_tv = {
+            .v_type = VAR_STRING,
+            .vval = { .v_string = (v.data.d.hi == NULL
+                                   ? v.data.d.dict->dv_hashtab.ht_array
+                                   : (v.data.d.hi - 1))->hi_key },
+        };
+        char *const key = tv2string(&key_tv, NULL);
+        vim_snprintf((char *) IObuff, IOSIZE, key_msg, key);
+        xfree(key);
+        ga_concat(&msg_ga, IObuff);
+        break;
+      }
+      case kMPConvPairs:
+      case kMPConvList: {
+        int idx = 0;
+        const listitem_T *li;
+        for (li = v.data.l.list->lv_first;
+             li != NULL && li->li_next != v.data.l.li;
+             li = li->li_next) {
+          idx++;
+        }
+        if (v.type == kMPConvList
+            || li == NULL
+            || (li->li_tv.v_type != VAR_LIST
+                && li->li_tv.vval.v_list->lv_len <= 0)) {
+          vim_snprintf((char *) IObuff, IOSIZE, idx_msg, idx);
+          ga_concat(&msg_ga, IObuff);
+        } else {
+          typval_T key_tv = li->li_tv.vval.v_list->lv_first->li_tv;
+          trylevel++;
+          typval_T rettv;
+          int doesrange;
+          char *key;
+          bool free_key = false;
+          if (call_func((char_u *) "msgpack#string",
+                        sizeof("msgpack#string") - 1,
+                        &rettv, 1, &key_tv, 0L, 0L, &doesrange, true,
+                        NULL) == FAIL
+              || ((key = (char *) get_tv_string(&rettv)) == NULL)
+              || did_throw
+              || (msg_list != NULL && *msg_list != NULL)) {
+            key = tv2string(&key_tv, NULL);
+            free_key = true;
+          }
+          did_emsg = false;
+          discard_current_exception();
+          if (msg_list != NULL && *msg_list != NULL) {
+            free_global_msglist();
+          }
+          trylevel--;
+          vim_snprintf((char *) IObuff, IOSIZE, key_pair_msg, key, idx);
+          clear_tv(&rettv);
+          if (free_key) {
+            xfree(key);
+          }
+          ga_concat(&msg_ga, IObuff);
+        }
+        break;
+      }
+    }
+  }
+  EMSG3(msg, objname, (kv_size(*mpstack) == 0
+                       ? _("itself")
+                       : (char *) msg_ga.ga_data));
+  ga_clear(&msg_ga);
+  return FAIL;
+}
+
 #define CONV_STRING(buf, len) \
     do { \
       if (buf == NULL) { \
@@ -12726,10 +12823,9 @@ static inline bool vim_list_to_buf(const list_T *const list,
     msgpack_pack_double(packer, (double) (flt))
 
 #define CONV_FUNC(fun) \
-    do { \
+    return conv_error(_("E951: Error while dumping %s, %s: " \
-      EMSG2(_(e_invarg2), "attempt to dump function reference"); \
+                        "attempt to dump function reference"), \
-      return FAIL; \
+                      mpstack, objname)
-    } while (0)
 
 #define CONV_EMPTY_LIST() \
     msgpack_pack_array(packer, 0)
@@ -12772,10 +12868,9 @@ static inline bool vim_list_to_buf(const list_T *const list,
 #define CONV_LIST_BETWEEN_ITEMS(lst)
 
 #define CONV_RECURSE(val, conv_type) \
-    do { \
+    return conv_error(_("E952: Unable to dump %s: " \
-      EMSG2(_(e_invarg2), "container references itself"); \
+                        "container references itself in %s"), \
-      return FAIL; \
+                      mpstack, objname)
-    } while (0)
 
 #define CONV_ALLOW_SPECIAL true
 
@@ -12817,8 +12912,14 @@ static void f_msgpackdump(typval_T *argvars, typval_T *rettv)
     return;
   }
   msgpack_packer *lpacker = msgpack_packer_new(ret_list, &msgpack_list_write);
+  const char *const msg = _("msgpackdump() argument, index %i");
+  // Assume that translation will not take more then 4 times more space
+  char msgbuf[sizeof("msgpackdump() argument, index ") * 4 + NUMBUFLEN];
+  int idx = 0;
   for (listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
-    if (vim_to_msgpack(lpacker, &li->li_tv) == FAIL) {
+    vim_snprintf(msgbuf, sizeof(msgbuf), (char *) msg, idx);
+    idx++;
+    if (vim_to_msgpack(lpacker, &li->li_tv, msgbuf) == FAIL) {
       break;
     }
   }

src/nvim/eval.h

@@ -118,7 +118,8 @@ enum {
 /// Maximum number of function arguments
 #define MAX_FUNC_ARGS   20
 
-int vim_to_msgpack(msgpack_packer *const, typval_T *const);
+int vim_to_msgpack(msgpack_packer *const, typval_T *const,
+                   const char *const objname);
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "eval.h.generated.h"

src/nvim/shada.c

@@ -1684,17 +1684,18 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
   msgpack_sbuffer sbuf;
   msgpack_sbuffer_init(&sbuf);
   msgpack_packer *spacker = msgpack_packer_new(&sbuf, &msgpack_sbuffer_write);
-#define DUMP_ADDITIONAL_ELEMENTS(src) \
+#define DUMP_ADDITIONAL_ELEMENTS(src, what) \
   do { \
     if ((src) != NULL) { \
       for (listitem_T *li = (src)->lv_first; li != NULL; li = li->li_next) { \
-        if (vim_to_msgpack(spacker, &li->li_tv) == FAIL) { \
+        if (vim_to_msgpack(spacker, &li->li_tv, \
+                           _("additional elements of ShaDa " what)) == FAIL) { \
           goto shada_pack_entry_error; \
         } \
       } \
     } \
   } while (0)
-#define DUMP_ADDITIONAL_DATA(src) \
+#define DUMP_ADDITIONAL_DATA(src, what) \
   do { \
     dict_T *const d = (src); \
     if (d != NULL) { \
@@ -1706,7 +1707,8 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
           const size_t key_len = strlen((const char *) hi->hi_key); \
           msgpack_pack_str(spacker, key_len); \
           msgpack_pack_str_body(spacker, (const char *) hi->hi_key, key_len); \
-          if (vim_to_msgpack(spacker, &di->di_tv) == FAIL) { \
+          if (vim_to_msgpack(spacker, &di->di_tv, \
+                             _("additional data of ShaDa " what)) == FAIL) { \
             goto shada_pack_entry_error; \
           } \
         } \
@@ -1741,7 +1743,8 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
       if (is_hist_search) {
         msgpack_pack_uint8(spacker, (uint8_t) entry.data.history_item.sep);
       }
-      DUMP_ADDITIONAL_ELEMENTS(entry.data.history_item.additional_elements);
+      DUMP_ADDITIONAL_ELEMENTS(entry.data.history_item.additional_elements,
+                               "history entry item");
       break;
     }
     case kSDItemVariable: {
@@ -1750,14 +1753,20 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
           ? 0
           : entry.data.global_var.additional_elements->lv_len);
       msgpack_pack_array(spacker, arr_size);
-      PACK_BIN(cstr_as_string(entry.data.global_var.name));
+      const String varname = cstr_as_string(entry.data.global_var.name);
-      if (vim_to_msgpack(spacker, &entry.data.global_var.value) == FAIL) {
+      PACK_BIN(varname);
+      char vardesc[256] = "variable g:";
+      memcpy(&vardesc[sizeof("variable g:") - 1], varname.data,
+             varname.size + 1);
+      if (vim_to_msgpack(spacker, &entry.data.global_var.value, vardesc)
+          == FAIL) {
         ret = kSDWriteIgnError;
         EMSG2(_(WERR "Failed to write variable %s"),
               entry.data.global_var.name);
         goto shada_pack_entry_error;
       }
-      DUMP_ADDITIONAL_ELEMENTS(entry.data.global_var.additional_elements);
+      DUMP_ADDITIONAL_ELEMENTS(entry.data.global_var.additional_elements,
+                               "variable item");
       break;
     }
     case kSDItemSubString: {
@@ -1767,7 +1776,8 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
           : entry.data.sub_string.additional_elements->lv_len);
       msgpack_pack_array(spacker, arr_size);
       PACK_BIN(cstr_as_string(entry.data.sub_string.sub));
-      DUMP_ADDITIONAL_ELEMENTS(entry.data.sub_string.additional_elements);
+      DUMP_ADDITIONAL_ELEMENTS(entry.data.sub_string.additional_elements,
+                               "sub string item");
       break;
     }
     case kSDItemSearchPattern: {
@@ -1814,7 +1824,8 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
         msgpack_pack_int64(spacker, entry.data.search_pattern.offset);
       }
 #undef PACK_BOOL
-      DUMP_ADDITIONAL_DATA(entry.data.search_pattern.additional_data);
+      DUMP_ADDITIONAL_DATA(entry.data.search_pattern.additional_data,
+                           "search pattern item");
       break;
     }
     case kSDItemChange:
@@ -1849,7 +1860,8 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
         PACK_STATIC_STR(KEY_NAME_CHAR);
         msgpack_pack_uint8(spacker, (uint8_t) entry.data.filemark.name);
       }
-      DUMP_ADDITIONAL_DATA(entry.data.filemark.additional_data);
+      DUMP_ADDITIONAL_DATA(entry.data.filemark.additional_data,
+                           "mark (change, jump, global or local) item");
       break;
     }
     case kSDItemRegister: {
@@ -1877,7 +1889,7 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
         PACK_STATIC_STR(REG_KEY_WIDTH);
         msgpack_pack_uint64(spacker, (uint64_t) entry.data.reg.width);
       }
-      DUMP_ADDITIONAL_DATA(entry.data.reg.additional_data);
+      DUMP_ADDITIONAL_DATA(entry.data.reg.additional_data, "register item");
       break;
     }
     case kSDItemBufferList: {
@@ -1908,7 +1920,8 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
           msgpack_pack_uint64(
               spacker, (uint64_t) entry.data.buffer_list.buffers[i].pos.col);
         }
-        DUMP_ADDITIONAL_DATA(entry.data.buffer_list.buffers[i].additional_data);
+        DUMP_ADDITIONAL_DATA(entry.data.buffer_list.buffers[i].additional_data,
+                             "buffer list subitem");
       }
       break;
     }