api: unify buffer numbers and window ids with handles

also allow handle==0 meaning curbuf/curwin/curtab

scripts/gendispatch.lua

@@ -193,9 +193,9 @@ for i = 1, #functions do
       output:write('\n  if (args.items['..(j - 1)..'].type == kObjectType'..rt..') {')
       output:write('\n    '..converted..' = args.items['..(j - 1)..'].data.'..rt:lower()..';')
       if rt:match('^Buffer$') or rt:match('^Window$') or rt:match('^Tabpage$') or rt:match('^Boolean$') then
-        -- accept positive integers for Buffers, Windows and Tabpages
+        -- accept nonnegative integers for Booleans, Buffers, Windows and Tabpages
-        output:write('\n  } else if (args.items['..(j - 1)..'].type == kObjectTypeInteger && args.items['..(j - 1)..'].data.integer > 0) {')
+        output:write('\n  } else if (args.items['..(j - 1)..'].type == kObjectTypeInteger && args.items['..(j - 1)..'].data.integer >= 0) {')
-        output:write('\n    '..converted..' = (unsigned)args.items['..(j - 1)..'].data.integer;')
+        output:write('\n    '..converted..' = (handle_T)args.items['..(j - 1)..'].data.integer;')
       end
       output:write('\n  } else {')
       output:write('\n    snprintf(error->msg, sizeof(error->msg), "Wrong type for argument '..j..', expecting '..param[1]..'");')

src/nvim/api/private/defs.h

@@ -9,13 +9,15 @@
 #define STRING_INIT {.data = NULL, .size = 0}
 #define OBJECT_INIT { .type = kObjectTypeNil }
 #define ERROR_INIT { .set = false }
-#define REMOTE_TYPE(type) typedef uint64_t type
+#define REMOTE_TYPE(type) typedef handle_T type
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # define ArrayOf(...) Array
 # define DictionaryOf(...) Dictionary
 #endif
 
+typedef int handle_T;
+
 // Basic types
 typedef enum {
   kErrorTypeException,

src/nvim/api/private/handle.c

@@ -5,30 +5,26 @@
 #include "nvim/map.h"
 #include "nvim/api/private/handle.h"
 
-#define HANDLE_INIT(name) name##_handles = pmap_new(uint64_t)()
+#define HANDLE_INIT(name) name##_handles = pmap_new(handle_T)()
 
 #define HANDLE_IMPL(type, name) \
-  static PMap(uint64_t) *name##_handles = NULL; \
+  static PMap(handle_T) *name##_handles = NULL; /* NOLINT */ \
   \
-  type *handle_get_##name(uint64_t handle) \
+  type *handle_get_##name(handle_T handle) \
   { \
-    return pmap_get(uint64_t)(name##_handles, handle); \
+    return pmap_get(handle_T)(name##_handles, handle); \
   } \
   \
   void handle_register_##name(type *name) \
   { \
-    assert(!name->handle); \
+    pmap_put(handle_T)(name##_handles, name->handle, name); \
-    name->handle = next_handle++; \
-    pmap_put(uint64_t)(name##_handles, name->handle, name); \
   } \
   \
   void handle_unregister_##name(type *name) \
   { \
-    pmap_del(uint64_t)(name##_handles, name->handle); \
+    pmap_del(handle_T)(name##_handles, name->handle); \
   }
 
-static uint64_t next_handle = 1;
-
 HANDLE_IMPL(buf_T, buffer)
 HANDLE_IMPL(win_T, window)
 HANDLE_IMPL(tabpage_T, tabpage)

src/nvim/api/private/handle.h

@@ -3,9 +3,10 @@
 
 #include "nvim/vim.h"
 #include "nvim/buffer_defs.h"
+#include "nvim/api/private/defs.h"
 
 #define HANDLE_DECLS(type, name) \
-  type *handle_get_##name(uint64_t handle); \
+  type *handle_get_##name(handle_T handle); \
   void handle_register_##name(type *name); \
   void handle_unregister_##name(type *name);
 

src/nvim/api/private/helpers.c

@@ -507,6 +507,10 @@ Object vim_to_object(typval_T *obj)
 
 buf_T *find_buffer_by_handle(Buffer buffer, Error *err)
 {
+  if (buffer == 0) {
+    return curbuf;
+  }
+
   buf_T *rv = handle_get_buffer(buffer);
 
   if (!rv) {
@@ -518,6 +522,10 @@ buf_T *find_buffer_by_handle(Buffer buffer, Error *err)
 
 win_T * find_window_by_handle(Window window, Error *err)
 {
+  if (window == 0) {
+    return curwin;
+  }
+
   win_T *rv = handle_get_window(window);
 
   if (!rv) {
@@ -529,6 +537,10 @@ win_T * find_window_by_handle(Window window, Error *err)
 
 tabpage_T * find_tab_by_handle(Tabpage tabpage, Error *err)
 {
+  if (tabpage == 0) {
+    return curtab;
+  }
+
   tabpage_T *rv = handle_get_tabpage(tabpage);
 
   if (!rv) {

src/nvim/api/vim.c

@@ -472,7 +472,7 @@ void vim_set_current_buffer(Buffer buffer, Error *err)
   if (!try_end(err) && result == FAIL) {
     api_set_error(err,
                   Exception,
-                  _("Failed to switch to buffer %" PRIu64),
+                  _("Failed to switch to buffer %d"),
                   buffer);
   }
 }
@@ -522,7 +522,7 @@ void vim_set_current_window(Window window, Error *err)
   if (!try_end(err) && win != curwin) {
     api_set_error(err,
                   Exception,
-                  _("Failed to switch to window %" PRIu64),
+                  _("Failed to switch to window %d"),
                   window);
   }
 }
@@ -573,7 +573,7 @@ void vim_set_current_tabpage(Tabpage tabpage, Error *err)
   if (!try_end(err) && tp != curtab) {
     api_set_error(err,
                   Exception,
-                  _("Failed to switch to tabpage %" PRIu64),
+                  _("Failed to switch to tabpage %d"),
                   tabpage);
   }
 }

src/nvim/buffer.c

@@ -1397,8 +1397,7 @@ buflist_new (
   }
   if (buf != curbuf || curbuf == NULL) {
     buf = xcalloc(1, sizeof(buf_T));
-    handle_register_buffer(buf);
+    // init b: variables
-    /* init b: variables */
     buf->b_vars = dict_alloc();
     init_var_dict(buf->b_vars, &buf->b_bufvar, VAR_SCOPE);
   }
@@ -1451,11 +1450,12 @@ buflist_new (
     lastbuf = buf;
 
     buf->b_fnum = top_file_num++;
-    if (top_file_num < 0) {             /* wrap around (may cause duplicates) */
+    handle_register_buffer(buf);
+    if (top_file_num < 0) {  // wrap around (may cause duplicates)
       EMSG(_("W14: Warning: List of file names overflow"));
       if (emsg_silent == 0) {
         ui_flush();
-        os_delay(3000L, true);          /* make sure it is noticed */
+        os_delay(3000L, true);  // make sure it is noticed
       }
       top_file_num = 1;
     }
@@ -5231,12 +5231,12 @@ wipe_buffer (
     int aucmd                   /* When TRUE trigger autocommands. */
 )
 {
-  if (buf->b_fnum == top_file_num - 1)
+  if (!aucmd) {
-    --top_file_num;
+    // Don't trigger BufDelete autocommands here.
-
-  if (!aucmd)               /* Don't trigger BufDelete autocommands here. */
     block_autocmds();
-  close_buffer(NULL, buf, DOBUF_WIPE, FALSE);
+  }
-  if (!aucmd)
+  close_buffer(NULL, buf, DOBUF_WIPE, false);
+  if (!aucmd) {
     unblock_autocmds();
   }
+}

src/nvim/buffer_defs.h

@@ -461,9 +461,10 @@ typedef struct {
  */
 
 struct file_buffer {
-  uint64_t handle;              // unique identifier for the buffer
+  handle_T handle;              // unique id for the buffer (buffer number)
-  memline_T b_ml;               /* associated memline (also contains line
+#define b_fnum handle
-                                   count) */
+
+  memline_T b_ml;               // associated memline (also contains line count
 
   buf_T       *b_next;          /* links in list of buffers */
   buf_T       *b_prev;
@@ -487,8 +488,6 @@ struct file_buffer {
   bool file_id_valid;
   FileID file_id;
 
-  int b_fnum;                   /* buffer number for this file. */
-
   bool b_changed;               /* 'modified': Set to true if something in the
                                    file has been changed and not written out. */
   int b_changedtick;            /* incremented for each change, also for undo */
@@ -799,28 +798,27 @@ struct diffblock_S {
 # define SNAP_AUCMD_IDX 1
 # define SNAP_COUNT     2
 
-/*
+/// Tab pages point to the top frame of each tab page.
- * Tab pages point to the top frame of each tab page.
+/// Note: Most values are NOT valid for the current tab page!  Use "curwin",
- * Note: Most values are NOT valid for the current tab page!  Use "curwin",
+/// "firstwin", etc. for that.  "tp_topframe" is always valid and can be
- * "firstwin", etc. for that.  "tp_topframe" is always valid and can be
+/// compared against "topframe" to find the current tab page.
- * compared against "topframe" to find the current tab page.
- */
 typedef struct tabpage_S tabpage_T;
 struct tabpage_S {
-  uint64_t handle;
+  handle_T handle;
-  tabpage_T       *tp_next;         /* next tabpage or NULL */
+  tabpage_T       *tp_next;         ///< next tabpage or NULL
-  frame_T         *tp_topframe;     /* topframe for the windows */
+  frame_T         *tp_topframe;     ///< topframe for the windows
-  win_T           *tp_curwin;       /* current window in this Tab page */
+  win_T           *tp_curwin;       ///< current window in this Tab page
-  win_T           *tp_prevwin;      /* previous window in this Tab page */
+  win_T           *tp_prevwin;      ///< previous window in this Tab page
-  win_T           *tp_firstwin;     /* first window in this Tab page */
+  win_T           *tp_firstwin;     ///< first window in this Tab page
-  win_T           *tp_lastwin;      /* last window in this Tab page */
+  win_T           *tp_lastwin;      ///< last window in this Tab page
-  long tp_old_Rows;                 /* Rows when Tab page was left */
+  long tp_old_Rows;                 ///< Rows when Tab page was left
-  long tp_old_Columns;              /* Columns when Tab page was left */
+  long tp_old_Columns;              ///< Columns when Tab page was left
-  long tp_ch_used;                  /* value of 'cmdheight' when frame size
+  long tp_ch_used;                  ///< value of 'cmdheight' when frame size
-                                       was set */
+                                    ///< was set
+
   diff_T          *tp_first_diff;
   buf_T           *(tp_diffbuf[DB_COUNT]);
-  int tp_diff_invalid;              ///< list of diffs is outdated */
+  int tp_diff_invalid;              ///< list of diffs is outdated
   frame_T         *(tp_snapshot[SNAP_COUNT]);    ///< window layout snapshots
   dictitem_T tp_winvar;             ///< variable for "t:" Dictionary
   dict_T          *tp_vars;         ///< internal variables, local to tab page
@@ -936,8 +934,8 @@ struct matchitem {
  * All row numbers are relative to the start of the window, except w_winrow.
  */
 struct window_S {
-  uint64_t handle;
+  handle_T handle;                  ///< unique identifier for the window
-  int w_id;                         ///< unique window ID
+
   buf_T       *w_buffer;            ///< buffer we are a window into (used
                                     ///< often, keep it the first item!)
 

src/nvim/map.c

@@ -20,6 +20,8 @@
 #define int_eq kh_int_hash_equal
 #define linenr_T_hash kh_int_hash_func
 #define linenr_T_eq kh_int_hash_equal
+#define handle_T_hash kh_int_hash_func
+#define handle_T_eq kh_int_hash_equal
 
 
 #if defined(ARCH_64)
@@ -141,6 +143,7 @@ MAP_IMPL(cstr_t, uint64_t, DEFAULT_INITIALIZER)
 MAP_IMPL(cstr_t, ptr_t, DEFAULT_INITIALIZER)
 MAP_IMPL(ptr_t, ptr_t, DEFAULT_INITIALIZER)
 MAP_IMPL(uint64_t, ptr_t, DEFAULT_INITIALIZER)
+MAP_IMPL(handle_T, ptr_t, DEFAULT_INITIALIZER)
 #define MSGPACK_HANDLER_INITIALIZER { .fn = NULL, .async = false }
 MAP_IMPL(String, MsgpackRpcRequestHandler, MSGPACK_HANDLER_INITIALIZER)
 #define KVEC_INITIALIZER { .size = 0, .capacity = 0, .items = NULL }

src/nvim/map.h

@@ -29,6 +29,7 @@ MAP_DECLS(cstr_t, uint64_t)
 MAP_DECLS(cstr_t, ptr_t)
 MAP_DECLS(ptr_t, ptr_t)
 MAP_DECLS(uint64_t, ptr_t)
+MAP_DECLS(handle_T, ptr_t)
 MAP_DECLS(String, MsgpackRpcRequestHandler)
 MAP_DECLS(linenr_T, bufhl_vec_T)
 

src/nvim/msgpack_rpc/helpers.c

@@ -40,7 +40,7 @@ static msgpack_sbuffer sbuffer;
       return false; \
     } \
     \
-    *arg = data.via.u64; \
+    *arg = (handle_T)data.via.i64; \
     return true; \
   } \
   \
@@ -49,7 +49,7 @@ static msgpack_sbuffer sbuffer;
   { \
     msgpack_packer pac; \
     msgpack_packer_init(&pac, &sbuffer, msgpack_sbuffer_write); \
-    msgpack_pack_uint64(&pac, o); \
+    msgpack_pack_int64(&pac, o); \
     msgpack_pack_ext(res, sbuffer.size, kObjectType##t); \
     msgpack_pack_ext_body(res, sbuffer.data, sbuffer.size); \
     msgpack_sbuffer_clear(&sbuffer); \

src/nvim/terminal.c

@@ -127,7 +127,7 @@ struct terminal {
   // we can't store a direct reference to the buffer because the
   // refresh_timer_cb may be called after the buffer was freed, and there's
   // no way to know if the memory was reused.
-  uint64_t buf_handle;
+  handle_T buf_handle;
   // program exited
   bool closed, destroy;
   // some vterm properties

src/nvim/window.c

@@ -2926,7 +2926,9 @@ void win_init_size(void)
  */
 static tabpage_T *alloc_tabpage(void)
 {
+  static int last_tp_handle = 0;
   tabpage_T *tp = xcalloc(1, sizeof(tabpage_T));
+  tp->handle = ++last_tp_handle;
   handle_register_tabpage(tp);
 
   /* init t: variables */
@@ -3683,21 +3685,20 @@ win_T *buf_jump_open_tab(buf_T *buf)
   return NULL;
 }
 
-static int last_win_id = 0;
-
 /*
  * Allocate a window structure and link it in the window list when "hidden" is
  * FALSE.
  */
 static win_T *win_alloc(win_T *after, int hidden)
 {
-  /*
+  static int last_win_id = 0;
-   * allocate window structure and linesizes arrays
+
-   */
+  // allocate window structure and linesizes arrays
   win_T *new_wp = xcalloc(1, sizeof(win_T));
-  handle_register_window(new_wp);
   win_alloc_lines(new_wp);
-  new_wp->w_id = ++last_win_id;
+
+  new_wp->handle = ++last_win_id;
+  handle_register_window(new_wp);
 
   /* init w: variables */
   new_wp->w_vars = dict_alloc();
@@ -5681,7 +5682,7 @@ static bool frame_check_width(frame_T *topfrp, int width)
 int win_getid(typval_T *argvars)
 {
   if (argvars[0].v_type == VAR_UNKNOWN) {
-    return curwin->w_id;
+    return curwin->handle;
   }
   int winnr = get_tv_number(&argvars[0]);
   win_T *wp;
@@ -5703,7 +5704,7 @@ int win_getid(typval_T *argvars)
     }
     for ( ; wp != NULL; wp = wp->w_next) {
       if (--winnr == 0) {
-        return wp->w_id;
+        return wp->handle;
       }
     }
   }
@@ -5719,7 +5720,7 @@ int win_gotoid(typval_T *argvars)
   for (tp = first_tabpage; tp != NULL; tp = tp->tp_next) {
     for (wp = tp == curtab ? firstwin : tp->tp_firstwin;
          wp != NULL; wp = wp->w_next) {
-      if (wp->w_id == id) {
+      if (wp->handle == id) {
         goto_tabpage_win(tp, wp);
         return 1;
       }
@@ -5739,7 +5740,7 @@ void win_id2tabwin(typval_T *argvars, list_T *list)
   for (tp = first_tabpage; tp != NULL; tp = tp->tp_next) {
     for (wp = tp == curtab ? firstwin : tp->tp_firstwin;
          wp != NULL; wp = wp->w_next) {
-      if (wp->w_id == id) {
+      if (wp->handle == id) {
         list_append_number(list, tabnr);
         list_append_number(list, winnr);
         return;
@@ -5760,7 +5761,7 @@ int win_id2win(typval_T *argvars)
   int id = get_tv_number(&argvars[0]);
 
   for (wp = firstwin; wp != NULL; wp = wp->w_next) {
-    if (wp->w_id == id) {
+    if (wp->handle == id) {
       return nr;
     }
     nr++;

test/functional/api/server_requests_spec.lua

@@ -163,11 +163,11 @@ describe('server -> client', function()
     end)
 
     it('can communicate buffers, tabpages, and windows', function()
-      eq({3}, eval("rpcrequest(vim, 'vim_get_tabpages')"))
+      eq({1}, eval("rpcrequest(vim, 'vim_get_tabpages')"))
       eq({1}, eval("rpcrequest(vim, 'vim_get_windows')"))
 
       local buf = eval("rpcrequest(vim, 'vim_get_buffers')")[1]
-      eq(2, buf)
+      eq(1, buf)
 
       eval("rpcnotify(vim, 'buffer_set_line', "..buf..", 0, 'SOME TEXT')")
       nvim('command', "call rpcrequest(vim, 'vim_eval', '0')")  -- wait

test/unit/buffer_spec.lua

@@ -87,7 +87,7 @@ describe('buffer functions', function()
     it('should find exact matches', function()
       local buf = buflist_new(path1, buffer.BLN_LISTED)
 
-      eq(buf.b_fnum, buflist_findpat(path1, ONLY_LISTED))
+      eq(buf.handle, buflist_findpat(path1, ONLY_LISTED))
 
       close_buffer(NULL, buf, buffer.DOBUF_WIPE, 0)
     end)
@@ -97,9 +97,9 @@ describe('buffer functions', function()
       local buf2 = buflist_new(path2, buffer.BLN_LISTED)
       local buf3 = buflist_new(path3, buffer.BLN_LISTED)
 
-      eq(buf1.b_fnum, buflist_findpat("test", ONLY_LISTED))
+      eq(buf1.handle, buflist_findpat("test", ONLY_LISTED))
-      eq(buf2.b_fnum, buflist_findpat("file", ONLY_LISTED))
+      eq(buf2.handle, buflist_findpat("file", ONLY_LISTED))
-      eq(buf3.b_fnum, buflist_findpat("path", ONLY_LISTED))
+      eq(buf3.handle, buflist_findpat("path", ONLY_LISTED))
 
       close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0)
       close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0)
@@ -113,7 +113,7 @@ describe('buffer functions', function()
       local buf3 = buflist_new(path3, buffer.BLN_LISTED)
 
       -- Then: buf2 is the buffer that is found
-      eq(buf2.b_fnum, buflist_findpat("test", ONLY_LISTED))
+      eq(buf2.handle, buflist_findpat("test", ONLY_LISTED))
       --}
 
       --{ When: We close buf2
@@ -123,7 +123,7 @@ describe('buffer functions', function()
       local buf1 = buflist_new(path1, buffer.BLN_LISTED)
 
       -- Then: buf3 is found since 'file' appears at the end of the name
-      eq(buf3.b_fnum, buflist_findpat("file", ONLY_LISTED))
+      eq(buf3.handle, buflist_findpat("file", ONLY_LISTED))
       --}
 
       close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0)
@@ -135,7 +135,7 @@ describe('buffer functions', function()
       local buf2 = buflist_new(path2, buffer.BLN_LISTED)
       local buf3 = buflist_new(path3, buffer.BLN_LISTED)
 
-      eq(buf3.b_fnum, buflist_findpat("_test_", ONLY_LISTED))
+      eq(buf3.handle, buflist_findpat("_test_", ONLY_LISTED))
 
       close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0)
       close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0)
@@ -147,7 +147,7 @@ describe('buffer functions', function()
       local buf3 = buflist_new(path3, buffer.BLN_LISTED)
 
       -- Then: We should find the buffer when it is given a unique pattern
-      eq(buf3.b_fnum, buflist_findpat("_test_", ONLY_LISTED))
+      eq(buf3.handle, buflist_findpat("_test_", ONLY_LISTED))
       --}
 
       --{ When: We unlist the buffer
@@ -157,7 +157,7 @@ describe('buffer functions', function()
       eq(-1, buflist_findpat("_test_", ONLY_LISTED))
 
       -- And: It should find the buffer when including unlisted buffers
-      eq(buf3.b_fnum, buflist_findpat("_test_", ALLOW_UNLISTED))
+      eq(buf3.handle, buflist_findpat("_test_", ALLOW_UNLISTED))
       --}
 
       --{ When: We wipe the buffer
@@ -175,7 +175,7 @@ describe('buffer functions', function()
       local buf2 = buflist_new(path2, buffer.BLN_LISTED)
 
       -- Then: The first buffer is preferred when both are listed
-      eq(buf1.b_fnum, buflist_findpat("test", ONLY_LISTED))
+      eq(buf1.handle, buflist_findpat("test", ONLY_LISTED))
       --}
 
       --{ When: The first buffer is unlisted
@@ -183,13 +183,13 @@ describe('buffer functions', function()
 
       -- Then: The second buffer is preferred because
       --       unlisted buffers are not allowed
-      eq(buf2.b_fnum, buflist_findpat("test", ONLY_LISTED))
+      eq(buf2.handle, buflist_findpat("test", ONLY_LISTED))
       --}
 
       --{ When: We allow unlisted buffers
       -- Then: The second buffer is still preferred
       --       because listed buffers are preferred to unlisted
-      eq(buf2.b_fnum, buflist_findpat("test", ALLOW_UNLISTED))
+      eq(buf2.handle, buflist_findpat("test", ALLOW_UNLISTED))
       --}
 
       --{ When: We unlist the second buffer
@@ -198,7 +198,7 @@ describe('buffer functions', function()
       -- Then: The first buffer is preferred again
       --       because buf1 matches better which takes precedence
       --       when both buffers have the same listing status.
-      eq(buf1.b_fnum, buflist_findpat("test", ALLOW_UNLISTED))
+      eq(buf1.handle, buflist_findpat("test", ALLOW_UNLISTED))
 
       -- And: Neither buffer is returned when ignoring unlisted
       eq(-1, buflist_findpat("test", ONLY_LISTED))