free_buffer: rework b:changedtick handling #9163

- Re-enable Test_BufLeave_Wipe. 66f5e5c7d7

This is my (probably-wrong) application of Zyx's suggestion in #9163:

> free_buffer_stuff() already removes changedtick. It is better to
> make `free_options` a flag variable and avoid calling
> buf_init_changedtick() based on some flag there: current workflow
> looks weird as it first removes `b:changedtick`, then re-adds it
> by calling buf_init_changedtick(), then remove again.

> Also based on my understanding it looks logical to not remove
> `b:changedtick`, but to *replace* it with something allocated if
> needed based on examining reference count before calling
> `unref_var_dict`. Because now you have key disappearing from
> dictionary for no good reason.

Patch-By: Nikolai Aleksandrovich Pavlov <kp-pav@yandex.ru>

src/nvim/buffer.c

@@ -97,6 +97,11 @@ static char *e_auabort = N_("E855: Autocommands caused command to abort");
 // Number of times free_buffer() was called.
 static int buf_free_count = 0;
 
+typedef enum {
+  kBffClearWinInfo = 1,
+  kBffInitChangedtick = 2,
+} BufFreeFlags;
+
 // Read data from buffer for retrying.
 static int
 read_buffer(
@@ -619,9 +624,9 @@ void close_buffer(win_T *win, buf_T *buf, int action, int abort_if_last)
     free_buffer(buf);
   } else {
     if (del_buf) {
-      /* Free all internal variables and reset option values, to make
+      // Free all internal variables and reset option values, to make
-       * ":bdel" compatible with Vim 5.7. */
+      // ":bdel" compatible with Vim 5.7.
-      free_buffer_stuff(buf, true);
+      free_buffer_stuff(buf, kBffClearWinInfo | kBffInitChangedtick);
 
       // Make it look like a new buffer.
       buf->b_flags = BF_CHECK_RO | BF_NEVERLOADED;
@@ -756,9 +761,12 @@ static void free_buffer(buf_T *buf)
 {
   handle_unregister_buffer(buf);
   buf_free_count++;
-  free_buffer_stuff(buf, true);
+  // b:changedtick uses an item in buf_T.
-  // b:changedtick uses an item in buf_T, remove it now.
+  free_buffer_stuff(buf, kBffClearWinInfo);
-  tv_dict_item_remove(buf->b_vars, (dictitem_T *)&buf->changedtick_di);
+  if (buf->b_vars->dv_refcount > DO_NOT_FREE_CNT) {
+    tv_dict_add(buf->b_vars,
+                tv_dict_item_copy((dictitem_T *)(&buf->changedtick_di)));
+  }
   unref_var_dict(buf->b_vars);
   aubuflocal_remove(buf);
   tv_dict_unref(buf->additional_data);
@@ -783,22 +791,19 @@ static void free_buffer(buf_T *buf)
   }
 }
 
-/*
+/// Free stuff in the buffer for ":bdel" and when wiping out the buffer.
- * Free stuff in the buffer for ":bdel" and when wiping out the buffer.
+///
- */
+/// @param buf  Buffer pointer
-static void
+/// @param free_flags  BufFreeFlags
-free_buffer_stuff(
+static void free_buffer_stuff(buf_T *buf, int free_flags)
-    buf_T *buf,
-    int free_options                       // free options as well
-)
 {
-  if (free_options) {
+  if (free_flags & kBffClearWinInfo) {
     clear_wininfo(buf);                 // including window-local options
     free_buf_options(buf, true);
     ga_clear(&buf->b_s.b_langp);
   }
   {
-    // Avoid loosing b:changedtick when deleting buffer: clearing variables
+    // Avoid losing b:changedtick when deleting buffer: clearing variables
     // implies using clear_tv() on b:changedtick and that sets changedtick to
     // zero.
     hashitem_T *const changedtick_hi = hash_find(
@@ -808,7 +813,9 @@ free_buffer_stuff(
   }
   vars_clear(&buf->b_vars->dv_hashtab);   // free all internal variables
   hash_init(&buf->b_vars->dv_hashtab);
-  buf_init_changedtick(buf);
+  if (free_flags & kBffInitChangedtick) {
+    buf_init_changedtick(buf);
+  }
   uc_clear(&buf->b_ucmds);               // clear local user commands
   buf_delete_signs(buf, (char_u *)"*");  // delete any signs
   bufhl_clear_all(buf);                  // delete any highligts
@@ -1787,7 +1794,7 @@ buf_T * buflist_new(char_u *ffname, char_u *sfname, linenr_T lnum, int flags)
     if (aborting()) {           // autocmds may abort script processing
       return NULL;
     }
-    free_buffer_stuff(buf, false);      // delete local variables et al.
+    free_buffer_stuff(buf, kBffInitChangedtick);  // delete local vars et al.
 
     // Init the options.
     buf->b_p_initialized = false;

src/nvim/eval/typval.c

@@ -1272,7 +1272,7 @@ void tv_dict_item_free(dictitem_T *const item)
 /// @param[in]  di  Item to copy.
 ///
 /// @return [allocated] new dictionary item.
-static dictitem_T *tv_dict_item_copy(dictitem_T *const di)
+dictitem_T *tv_dict_item_copy(dictitem_T *const di)
   FUNC_ATTR_NONNULL_RET FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
   dictitem_T *const new_di = tv_dict_item_alloc((const char *)di->di_key);

src/nvim/testdir/test_autocmd.vim

@@ -811,7 +811,6 @@ endfunc
 " Test for autocommand that deletes the current buffer on BufLeave event.
 " Also test deleting the last buffer, should give a new, empty buffer.
 func Test_BufLeave_Wipe()
-  throw 'skipped: TODO: '
   %bwipe!
   let content = ['start of test file Xxx',
 	      \ 'this is a test',