eval: Move part of dictwatcher* functions to eval/typval

src/nvim/eval.c

@@ -7706,6 +7706,11 @@ static void f_dictwatcheradd(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   if (argvars[0].v_type != VAR_DICT) {
     emsgf(_(e_invarg2), "dict");
     return;
+  } else if (argvars[0].vval.v_dict == NULL) {
+    const char *const arg_errmsg = _("dictwatcheradd() argument");
+    const size_t arg_errmsg_len = strlen(arg_errmsg);
+    emsgf(_(e_readonlyvar), (int)arg_errmsg_len, arg_errmsg);
+    return;
   }
 
   if (argvars[1].v_type != VAR_STRING && argvars[1].v_type != VAR_NUMBER) {
@@ -7725,11 +7730,8 @@ static void f_dictwatcheradd(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     return;
   }
 
-  DictWatcher *watcher = xmalloc(sizeof(DictWatcher));
+  tv_dict_watcher_add(argvars[0].vval.v_dict, key_pattern, key_pattern_len,
-  watcher->key_pattern = xmemdupz(key_pattern, key_pattern_len);
+                      callback);
-  watcher->callback = callback;
-  watcher->busy = false;
-  QUEUE_INSERT_TAIL(&argvars[0].vval.v_dict->watchers, &watcher->node);
 }
 
 // dictwatcherdel(dict, key, funcref) function
@@ -7759,28 +7761,12 @@ static void f_dictwatcherdel(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     return;
   }
 
-  dict_T *dict = argvars[0].vval.v_dict;
+  if (!tv_dict_watcher_remove(argvars[0].vval.v_dict, key_pattern,
-  QUEUE *w = NULL;
+                              strlen(key_pattern), callback)) {
-  DictWatcher *watcher = NULL;
+    EMSG("Couldn't find a watcher matching key and callback");
-  bool matched = false;
-  QUEUE_FOREACH(w, &dict->watchers) {
-    watcher = tv_dict_watcher_node_data(w);
-    if (callback_equal(&watcher->callback, &callback)
-        && !strcmp(watcher->key_pattern, key_pattern)) {
-      matched = true;
-      break;
-    }
   }
 
   callback_free(&callback);
-
-  if (!matched) {
-    EMSG("Couldn't find a watcher matching key and callback");
-    return;
-  }
-
-  QUEUE_REMOVE(w);
-  tv_dict_watcher_free(watcher);
 }
 
 /*
@@ -16576,7 +16562,6 @@ bool callback_from_typval(Callback *const callback, typval_T *const arg)
   return true;
 }
 
-
 /// Unref/free callback
 void callback_free(Callback *const callback)
   FUNC_ATTR_NONNULL_ALL
@@ -16601,28 +16586,6 @@ void callback_free(Callback *const callback)
   callback->type = kCallbackNone;
 }
 
-static bool callback_equal(Callback *cb1, Callback *cb2)
-{
-  if (cb1->type != cb2->type) {
-    return false;
-  }
-  switch (cb1->type) {
-    case kCallbackFuncref:
-      return STRCMP(cb1->data.funcref, cb2->data.funcref) == 0;
-
-    case kCallbackPartial:
-      // FIXME: this is inconsistent with tv_equal but is needed for precision
-      // maybe change dictwatcheradd to return a watcher id instead?
-      return cb1->data.partial == cb2->data.partial;
-
-    case kCallbackNone:
-      return true;
-
-    default:
-      abort();
-  }
-}
-
 bool callback_call(Callback *const callback, const int argcount_in,
                    typval_T *const argvars_in, typval_T *const rettv)
   FUNC_ATTR_NONNULL_ALL

src/nvim/eval/typval.c

@@ -790,7 +790,7 @@ long tv_list_idx_of_item(const list_T *const l, const listitem_T *const item)
 /// Perform all necessary cleanup for a `DictWatcher` instance
 ///
 /// @param  watcher  Watcher to free.
-void tv_dict_watcher_free(DictWatcher *watcher)
+static void tv_dict_watcher_free(DictWatcher *watcher)
   FUNC_ATTR_NONNULL_ALL
 {
   callback_free(&watcher->callback);
@@ -798,6 +798,91 @@ void tv_dict_watcher_free(DictWatcher *watcher)
   xfree(watcher);
 }
 
+/// Add watcher to a dictionary
+///
+/// @param[in]  dict  Dictionary to add watcher to.
+/// @param[in]  key_pattern  Pattern to watch for.
+/// @param[in]  key_pattern_len  Key pattern length.
+/// @param  callback  Function to be called on events.
+void tv_dict_watcher_add(dict_T *const dict, const char *const key_pattern,
+                         const size_t key_pattern_len, Callback callback)
+  FUNC_ATTR_NONNULL_ARG(2)
+{
+  DictWatcher *const watcher = xmalloc(sizeof(DictWatcher));
+  watcher->key_pattern = xmemdupz(key_pattern, key_pattern_len);
+  watcher->key_pattern_len = key_pattern_len;
+  watcher->callback = callback;
+  watcher->busy = false;
+  QUEUE_INSERT_TAIL(&dict->watchers, &watcher->node);
+}
+
+/// Check whether two callbacks are equal
+///
+/// @param[in]  cb1  First callback to check.
+/// @param[in]  cb2  Second callback to check.
+///
+/// @return True if they are equal, false otherwise.
+static bool tv_callback_equal(const Callback *const cb1,
+                              const Callback *const cb2)
+  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+{
+  if (cb1->type != cb2->type) {
+    return false;
+  }
+  switch (cb1->type) {
+    case kCallbackFuncref: {
+      return STRCMP(cb1->data.funcref, cb2->data.funcref) == 0;
+    }
+    case kCallbackPartial: {
+      // FIXME: this is inconsistent with tv_equal but is needed for precision
+      // maybe change dictwatcheradd to return a watcher id instead?
+      return cb1->data.partial == cb2->data.partial;
+    }
+    case kCallbackNone: {
+      return true;
+    }
+  }
+}
+
+/// Remove watcher from a dictionary
+///
+/// @param  dict  Dictionary to remove watcher from.
+/// @param[in]  key_pattern  Pattern to remove watcher for.
+/// @param[in]  key_pattern_len  Pattern length.
+/// @param  callback  Callback to remove watcher for.
+///
+/// @return True on success, false if relevant watcher was not found.
+bool tv_dict_watcher_remove(dict_T *const dict, const char *const key_pattern,
+                            const size_t key_pattern_len,
+                            Callback callback)
+  FUNC_ATTR_NONNULL_ARG(2)
+{
+  if (dict == NULL) {
+    return false;
+  }
+
+  QUEUE *w = NULL;
+  DictWatcher *watcher = NULL;
+  bool matched = false;
+  QUEUE_FOREACH(w, &dict->watchers) {
+    watcher = tv_dict_watcher_node_data(w);
+    if (tv_callback_equal(&watcher->callback, &callback)
+        && watcher->key_pattern_len == key_pattern_len
+        && memcmp(watcher->key_pattern, key_pattern, key_pattern_len) == 0) {
+      matched = true;
+      break;
+    }
+  }
+
+  if (!matched) {
+    return false;
+  }
+
+  QUEUE_REMOVE(w);
+  tv_dict_watcher_free(watcher);
+  return true;
+}
+
 /// Test if `key` matches with with `watcher->key_pattern`
 ///
 /// @param[in]  watcher  Watcher to check key pattern from.
@@ -810,7 +895,7 @@ static bool tv_dict_watcher_matches(DictWatcher *watcher, const char *const key)
   // For now only allow very simple globbing in key patterns: a '*' at the end
   // of the string means it should match everything up to the '*' instead of the
   // whole string.
-  const size_t len = strlen(watcher->key_pattern);
+  const size_t len = watcher->key_pattern_len;
   if (len && watcher->key_pattern[len - 1] == '*') {
     return strncmp(key, watcher->key_pattern, len - 1) == 0;
   } else {

src/nvim/eval/typval.h

@@ -59,6 +59,7 @@ typedef struct {
 typedef struct dict_watcher {
   Callback callback;
   char *key_pattern;
+  size_t key_pattern_len;
   QUEUE node;
   bool busy;  // prevent recursion if the dict is changed in the callback
 } DictWatcher;
@@ -322,19 +323,6 @@ static inline bool tv_dict_is_watched(const dict_T *const d)
   return d && !QUEUE_EMPTY(&d->watchers);
 }
 
-static inline DictWatcher *tv_dict_watcher_node_data(QUEUE *q)
-  REAL_FATTR_NONNULL_ALL REAL_FATTR_NONNULL_RET REAL_FATTR_PURE
-  REAL_FATTR_WARN_UNUSED_RESULT;
-
-/// Compute the `DictWatcher` address from a QUEUE node.
-///
-/// This only exists for .asan-blacklist (ASAN doesn't handle QUEUE_DATA pointer
-/// arithmetic).
-static inline DictWatcher *tv_dict_watcher_node_data(QUEUE *q)
-{
-  return QUEUE_DATA(q, DictWatcher, node);
-}
-
 /// Initialize VimL object
 ///
 /// Initializes to unlocked VAR_UNKNOWN object.
@@ -407,6 +395,19 @@ static inline bool tv_get_float_chk(const typval_T *const tv,
   return false;
 }
 
+static inline DictWatcher *tv_dict_watcher_node_data(QUEUE *q)
+  REAL_FATTR_NONNULL_ALL REAL_FATTR_NONNULL_RET REAL_FATTR_PURE
+  REAL_FATTR_WARN_UNUSED_RESULT REAL_FATTR_ALWAYS_INLINE;
+
+/// Compute the `DictWatcher` address from a QUEUE node.
+///
+/// This only exists for .asan-blacklist (ASAN doesn't handle QUEUE_DATA pointer
+/// arithmetic).
+static inline DictWatcher *tv_dict_watcher_node_data(QUEUE *q)
+{
+  return QUEUE_DATA(q, DictWatcher, node);
+}
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "eval/typval.h.generated.h"
 #endif

src/nvim/os/env.c

@@ -18,7 +18,6 @@
 #include "nvim/eval.h"
 #include "nvim/ex_getln.h"
 #include "nvim/version.h"
-#include "nvim/fileio.h"
 
 #ifdef WIN32
 #include "nvim/mbyte.h"  // for utf8_to_utf16, utf16_to_utf8

test/functional/eval/null_spec.lua

@@ -80,8 +80,7 @@ describe('NULL', function()
     null_list_test('is accepted by :cexpr', 'cexpr L', 'Vim(cexpr):E777: String or List expected')
     -- FIXME should not error out
     null_list_test('is accepted by :lexpr', 'lexpr L', 'Vim(lexpr):E777: String or List expected')
-    -- FIXME should not error out
+    null_list_test('is accepted by :for', 'for x in L|throw x|endfor', 0)
-    null_list_test('is accepted by :for', 'for x in L|throw x|endfor', 'Vim(for):E714: List required')
 
     -- Subjectable behaviour