eval: Add id() function and make printf("%p") return something useful (#6095)

7 files changed, 113 insertions, 11 deletions

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index b729519d93..a14221a656 100644
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -2036,6 +2036,7 @@ insert({list}, {item} [, {idx}])
 invert({expr})			Number  bitwise invert
 isdirectory({directory})	Number	TRUE if {directory} is a directory
 islocked({expr})		Number	TRUE if {expr} is locked
+id({expr})			String	identifier of the container
 items({dict})			List	key-value pairs in {dict}
 jobclose({job}[, {stream}])	Number	Closes a job stream(s)
 jobpid({job})			Number	Returns pid of a job.
@@ -4629,6 +4630,22 @@ islocked({expr})					*islocked()* *E786*
 <		When {expr} is a variable that does not exist you get an error
 		message.  Use |exists()| to check for existence.
 
+id({expr})						*id()*
+		Returns a |String| which is a unique identifier of the 
+		container type (|List|, |Dict| and |Partial|). It is 
+		guaranteed that for the mentioned types `id(v1) ==# id(v2)` 
+		returns true iff `type(v1) == type(v2) && v1 is v2` (note: 
+		|v:_null_list| and |v:_null_dict| have the same `id()` with 
+		different types because they are internally represented as 
+		a NULL pointers). Currently `id()` returns a hexadecimal 
+		representanion of the pointers to the containers (i.e. like 
+		`0x994a40`), same as `printf("%p", {expr})`, but it is advised 
+		against counting on exact format of return value.
+
+		It is not guaranteed that `id(no_longer_existing_container)` 
+		will not be equal to some other `id()`: new containers may 
+		reuse identifiers of the garbage-collected ones.
+
 items({dict})						*items()*
 		Return a |List| with all the key-value pairs of {dict}.  Each
 		|List| item is a list with two items: the key of a {dict}
@@ -5500,6 +5517,7 @@ printf({fmt}, {expr1} ...)				*printf()*
 		  %g	floating point number, as %f or %e depending on value
 		  %G	floating point number, as %f or %E depending on value
 		  %%	the % character itself
+		  %p	representation of the pointer to the container
 
 		Conversion specifications start with '%' and end with the
 		conversion type.  All other characters are copied unchanged to
diff --git a/runtime/doc/vim_diff.txt b/runtime/doc/vim_diff.txt
index de93aab399..eeb5e85036 100644
--- a/runtime/doc/vim_diff.txt
+++ b/runtime/doc/vim_diff.txt
@@ -232,6 +232,12 @@ Additional differences:
   itself.
 - ShaDa file keeps search direction (|v:searchforward|), viminfo does not.
 
+|printf()| returns something meaningful when used with `%p` argument: in Vim 
+it used to return useless address of the string (strings are copied to the 
+newly allocated memory all over the place) and fail on types which cannot be 
+coerced to strings. See |id()| for more details, currently it uses 
+`printf("%p", {expr})` internally.
+
 ==============================================================================
 5. Missing legacy features				 *nvim-features-missing*
 				     *if_lua* *if_perl* *if_mzscheme* *if_tcl*
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 823b621638..bbb6565509 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -498,6 +498,14 @@ static PMap(uint64_t) *jobs = NULL;
 static uint64_t last_timer_id = 0;
 static PMap(uint64_t) *timers = NULL;
 
+/// Dummy va_list for passing to vim_snprintf
+///
+/// Used because:
+/// - passing a NULL pointer doesn't work when va_list isn't a pointer
+/// - locally in the function results in a "used before set" warning
+/// - using va_start() to initialize it gives "function with fixed args" error
+static va_list dummy_ap;
+
 static const char *const msgpack_type_names[] = {
   [kMPNil] = "nil",
   [kMPBoolean] = "boolean",
@@ -12122,6 +12130,16 @@ static void dict_list(typval_T *argvars, typval_T *rettv, int what)
   }
 }
 
+/// "id()" function
+static void f_id(typval_T *argvars, typval_T *rettv, FunPtr fptr)
+  FUNC_ATTR_NONNULL_ALL
+{
+  const int len = vim_vsnprintf(NULL, 0, "%p", dummy_ap, argvars);
+  rettv->v_type = VAR_STRING;
+  rettv->vval.v_string = xmalloc(len + 1);
+  vim_vsnprintf((char *)rettv->vval.v_string, len + 1, "%p", dummy_ap, argvars);
+}
+
 /*
  * "items(dict)" function
  */
@@ -13628,12 +13646,6 @@ static void f_prevnonblank(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   rettv->vval.v_number = lnum;
 }
 
-/* This dummy va_list is here because:
- * - passing a NULL pointer doesn't work when va_list isn't a pointer
- * - locally in the function results in a "used before set" warning
- * - using va_start() to initialize it gives "function with fixed args" error */
-static va_list ap;
-
 /*
  * "printf()" function
  */
@@ -13650,11 +13662,11 @@ static void f_printf(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     /* Get the required length, allocate the buffer and do it for real. */
     did_emsg = FALSE;
     fmt = (char *)get_tv_string_buf(&argvars[0], buf);
-    len = vim_vsnprintf(NULL, 0, fmt, ap, argvars + 1);
+    len = vim_vsnprintf(NULL, 0, fmt, dummy_ap, argvars + 1);
     if (!did_emsg) {
       char *s = xmalloc(len + 1);
       rettv->vval.v_string = (char_u *)s;
-      (void)vim_vsnprintf(s, len + 1, fmt, ap, argvars + 1);
+      (void)vim_vsnprintf(s, len + 1, fmt, dummy_ap, argvars + 1);
     }
     did_emsg |= saved_did_emsg;
   }
diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua
index e0b72feb19..964b061e95 100644
--- a/src/nvim/eval.lua
+++ b/src/nvim/eval.lua
@@ -168,6 +168,7 @@ return {
     invert={args=1},
     isdirectory={args=1},
     islocked={args=1},
+    id={args=1},
     items={args=1},
     jobclose={args={1, 2}},
     jobpid={args=1},
diff --git a/src/nvim/eval_defs.h b/src/nvim/eval_defs.h
index f478d19ca1..616c89671b 100644
--- a/src/nvim/eval_defs.h
+++ b/src/nvim/eval_defs.h
@@ -49,7 +49,7 @@ typedef enum {
 typedef struct {
   VarType v_type;  ///< Variable type.
   VarLockStatus v_lock;  ///< Variable lock status.
-  union {
+  union typval_vval_union {
     varnumber_T v_number;  ///< Number, for VAR_NUMBER.
     SpecialVarValue v_special;  ///< Special value, for VAR_SPECIAL.
     float_T v_float;  ///< Floating-point number, for VAR_FLOAT.
diff --git a/src/nvim/message.c b/src/nvim/message.c
index 6104adf2c7..ad1c63eeac 100644
--- a/src/nvim/message.c
+++ b/src/nvim/message.c
@@ -11,6 +11,7 @@
 
 #include "nvim/vim.h"
 #include "nvim/ascii.h"
+#include "nvim/assert.h"
 #include "nvim/message.h"
 #include "nvim/charset.h"
 #include "nvim/eval.h"
@@ -3043,6 +3044,38 @@ static char *tv_str(typval_T *tvs, int *idxp)
   return s;
 }
 
+/// Get pointer argument from the next entry in tvs
+///
+/// First entry is 1. Returns NULL for an error.
+///
+/// @param[in]  tvs  List of typval_T values.
+/// @param[in,out]  idxp  Pointer to the index of the current value.
+///
+/// @return Pointer stored in typval_T or NULL.
+static const void *tv_ptr(const typval_T *const tvs, int *const idxp)
+  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+{
+#define OFF(attr) offsetof(union typval_vval_union, attr)
+  STATIC_ASSERT(
+      OFF(v_string) == OFF(v_list)
+      && OFF(v_string) == OFF(v_dict)
+      && OFF(v_string) == OFF(v_partial)
+      && sizeof(tvs[0].vval.v_string) == sizeof(tvs[0].vval.v_list)
+      && sizeof(tvs[0].vval.v_string) == sizeof(tvs[0].vval.v_dict)
+      && sizeof(tvs[0].vval.v_string) == sizeof(tvs[0].vval.v_partial),
+      "Strings, dictionaries, lists and partials are expected to be pointers, "
+      "so that all three of them can be accessed via v_string");
+#undef OFF
+  const int idx = *idxp - 1;
+  if (tvs[idx].v_type == VAR_UNKNOWN) {
+    EMSG(_(e_printf));
+    return NULL;
+  } else {
+    (*idxp)++;
+    return tvs[idx].vval.v_string;
+  }
+}
+
 /*
  * Get float argument from "idxp" entry in "tvs".  First entry is 1.
  */
@@ -3369,11 +3402,11 @@ int vim_vsnprintf(char *str, size_t str_m, const char *fmt, va_list ap,
         size_t size_t_arg = 0;
 
         // only defined for p conversion
-        void *ptr_arg = NULL;
+        const void *ptr_arg = NULL;
 
         if (fmt_spec == 'p') {
           length_modifier = '\0';
-          ptr_arg = tvs ? (void *)tv_str(tvs, &arg_idx) : va_arg(ap, void *);
+          ptr_arg = tvs ? tv_ptr(tvs, &arg_idx) : va_arg(ap, void *);
           if (ptr_arg) {
             arg_sign = 1;
           }
diff --git a/test/functional/eval/printf_spec.lua b/test/functional/eval/printf_spec.lua
index c84290ceef..27e24c4118 100644
--- a/test/functional/eval/printf_spec.lua
+++ b/test/functional/eval/printf_spec.lua
@@ -1,7 +1,10 @@
 local helpers = require('test.functional.helpers')(after_each)
+
 local clear = helpers.clear
 local eq = helpers.eq
+local eval = helpers.eval
 local funcs = helpers.funcs
+local meths = helpers.meths
 local exc_exec = helpers.exc_exec
 
 describe('printf()', function()
@@ -57,4 +60,33 @@ describe('printf()', function()
   it('errors out when %b modifier is used for a float', function()
     eq('Vim(call):E805: Using a Float as a Number', exc_exec('call printf("%b", 3.1415926535)'))
   end)
+  it('works with %p correctly', function()
+    local null_ret = nil
+    local seen_rets = {}
+    -- Collect all args in an array to avoid possible allocation of the same
+    -- address after freeing unreferenced values.
+    meths.set_var('__args', {})
+    local function check_printf(expr, is_null)
+      eq(0, exc_exec('call add(__args, ' .. expr .. ')'))
+      eq(0, exc_exec('let __result = printf("%p", __args[-1])'))
+      local id_ret = eval('id(__args[-1])')
+      eq(id_ret, meths.get_var('__result'))
+      if is_null then
+        if null_ret then
+          eq(null_ret, id_ret)
+        else
+          null_ret = id_ret
+        end
+      else
+        eq(nil, seen_rets[id_ret])
+        seen_rets[id_ret] = expr
+      end
+      meths.del_var('__result')
+    end
+    check_printf('v:_null_list', true)
+    check_printf('v:_null_dict', true)
+    check_printf('[]')
+    check_printf('{}')
+    check_printf('function("tr", ["a"])')
+  end)
 end)