Merge pull request #27428 from bfredl/luarena

refactor(lua): use Arena when converting from lua stack to API args

4 files changed, 183 insertions, 204 deletions

diff --git a/src/nvim/lua/converter.c b/src/nvim/lua/converter.c
index 3cf5b0e94f..423d2fa775 100644
--- a/src/nvim/lua/converter.c
+++ b/src/nvim/lua/converter.c
@@ -795,8 +795,8 @@ void nlua_push_Object(lua_State *lstate, const Object obj, bool special)
 /// Convert lua value to string
 ///
 /// Always pops one value from the stack.
-String nlua_pop_String(lua_State *lstate, Error *err)
-  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+String nlua_pop_String(lua_State *lstate, Arena *arena, Error *err)
+  FUNC_ATTR_NONNULL_ARG(1, 3) FUNC_ATTR_WARN_UNUSED_RESULT
 {
   if (lua_type(lstate, -1) != LUA_TSTRING) {
     lua_pop(lstate, 1);
@@ -807,7 +807,10 @@ String nlua_pop_String(lua_State *lstate, Error *err)
 
   ret.data = (char *)lua_tolstring(lstate, -1, &(ret.size));
   assert(ret.data != NULL);
-  ret.data = xmemdupz(ret.data, ret.size);
+  // TODO(bfredl): it would be "nice" to just use the memory of the lua string
+  // directly, although ensuring the lifetime of such strings is a bit tricky
+  // (an API call could invoke nested lua, which triggers GC, and kaboom?)
+  ret.data = arena_memdupz(arena, ret.data, ret.size);
   lua_pop(lstate, 1);
 
   return ret;
@@ -816,8 +819,8 @@ String nlua_pop_String(lua_State *lstate, Error *err)
 /// Convert lua value to integer
 ///
 /// Always pops one value from the stack.
-Integer nlua_pop_Integer(lua_State *lstate, Error *err)
-  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+Integer nlua_pop_Integer(lua_State *lstate, Arena *arena, Error *err)
+  FUNC_ATTR_NONNULL_ARG(1, 3) FUNC_ATTR_WARN_UNUSED_RESULT
 {
   if (lua_type(lstate, -1) != LUA_TNUMBER) {
     lua_pop(lstate, 1);
@@ -840,8 +843,8 @@ Integer nlua_pop_Integer(lua_State *lstate, Error *err)
 /// thus `err` is never set as any lua value can be co-erced into a lua bool
 ///
 /// Always pops one value from the stack.
-Boolean nlua_pop_Boolean(lua_State *lstate, Error *err)
-  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+Boolean nlua_pop_Boolean(lua_State *lstate, Arena *arena, Error *err)
+  FUNC_ATTR_NONNULL_ARG(1, 3) FUNC_ATTR_WARN_UNUSED_RESULT
 {
   const Boolean ret = lua_toboolean(lstate, -1);
   lua_pop(lstate, 1);
@@ -915,7 +918,7 @@ static inline LuaTableProps nlua_check_type(lua_State *const lstate, Error *cons
 /// Convert lua table to float
 ///
 /// Always pops one value from the stack.
-Float nlua_pop_Float(lua_State *lstate, Error *err)
+Float nlua_pop_Float(lua_State *lstate, Arena *arena, Error *err)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
   if (lua_type(lstate, -1) == LUA_TNUMBER) {
@@ -939,29 +942,29 @@ Float nlua_pop_Float(lua_State *lstate, Error *err)
 /// @param[in]  table_props  nlua_traverse_table() output.
 /// @param[out]  err  Location where error will be saved.
 static Array nlua_pop_Array_unchecked(lua_State *const lstate, const LuaTableProps table_props,
-                                      Error *const err)
+                                      Arena *arena, Error *const err)
 {
-  Array ret = { .size = table_props.maxidx, .items = NULL };
+  Array ret = arena_array(arena, table_props.maxidx);
 
-  if (ret.size == 0) {
+  if (table_props.maxidx == 0) {
     lua_pop(lstate, 1);
     return ret;
   }
 
-  ret.items = xcalloc(ret.size, sizeof(*ret.items));
-  for (size_t i = 1; i <= ret.size; i++) {
+  for (size_t i = 1; i <= table_props.maxidx; i++) {
     Object val;
 
     lua_rawgeti(lstate, -1, (int)i);
 
-    val = nlua_pop_Object(lstate, false, err);
+    val = nlua_pop_Object(lstate, false, arena, err);
     if (ERROR_SET(err)) {
-      ret.size = i - 1;
       lua_pop(lstate, 1);
-      api_free_array(ret);
+      if (!arena) {
+        api_free_array(ret);
+      }
       return (Array) { .size = 0, .items = NULL };
     }
-    ret.items[i - 1] = val;
+    ADD_C(ret, val);
   }
   lua_pop(lstate, 1);
 
@@ -971,15 +974,14 @@ static Array nlua_pop_Array_unchecked(lua_State *const lstate, const LuaTablePro
 /// Convert lua table to array
 ///
 /// Always pops one value from the stack.
-Array nlua_pop_Array(lua_State *lstate, Error *err)
-  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+Array nlua_pop_Array(lua_State *lstate, Arena *arena, Error *err)
+  FUNC_ATTR_NONNULL_ARG(1, 3) FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  const LuaTableProps table_props = nlua_check_type(lstate, err,
-                                                    kObjectTypeArray);
+  const LuaTableProps table_props = nlua_check_type(lstate, err, kObjectTypeArray);
   if (table_props.type != kObjectTypeArray) {
     return (Array) { .size = 0, .items = NULL };
   }
-  return nlua_pop_Array_unchecked(lstate, table_props, err);
+  return nlua_pop_Array_unchecked(lstate, table_props, arena, err);
 }
 
 /// Convert lua table to dictionary
@@ -991,30 +993,30 @@ Array nlua_pop_Array(lua_State *lstate, Error *err)
 /// @param[in]  table_props  nlua_traverse_table() output.
 /// @param[out]  err  Location where error will be saved.
 static Dictionary nlua_pop_Dictionary_unchecked(lua_State *lstate, const LuaTableProps table_props,
-                                                bool ref, Error *err)
-  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+                                                bool ref, Arena *arena, Error *err)
+  FUNC_ATTR_NONNULL_ARG(1, 5) FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  Dictionary ret = { .size = table_props.string_keys_num, .items = NULL };
+  Dictionary ret = arena_dict(arena, table_props.string_keys_num);
 
-  if (ret.size == 0) {
+  if (table_props.string_keys_num == 0) {
     lua_pop(lstate, 1);
     return ret;
   }
-  ret.items = xcalloc(ret.size, sizeof(*ret.items));
 
   lua_pushnil(lstate);
-  for (size_t i = 0; lua_next(lstate, -2) && i < ret.size;) {
+  for (size_t i = 0; lua_next(lstate, -2) && i < table_props.string_keys_num;) {
     // stack: dict, key, value
 
     if (lua_type(lstate, -2) == LUA_TSTRING) {
       lua_pushvalue(lstate, -2);
       // stack: dict, key, value, key
 
-      ret.items[i].key = nlua_pop_String(lstate, err);
+      String key = nlua_pop_String(lstate, arena, err);
       // stack: dict, key, value
 
       if (!ERROR_SET(err)) {
-        ret.items[i].value = nlua_pop_Object(lstate, ref, err);
+        Object value = nlua_pop_Object(lstate, ref, arena, err);
+        kv_push_c(ret, ((KeyValuePair) { .key = key, .value = value }));
         // stack: dict, key
       } else {
         lua_pop(lstate, 1);
@@ -1022,8 +1024,9 @@ static Dictionary nlua_pop_Dictionary_unchecked(lua_State *lstate, const LuaTabl
       }
 
       if (ERROR_SET(err)) {
-        ret.size = i;
-        api_free_dictionary(ret);
+        if (!arena) {
+          api_free_dictionary(ret);
+        }
         lua_pop(lstate, 2);
         // stack:
         return (Dictionary) { .size = 0, .items = NULL };
@@ -1042,8 +1045,8 @@ static Dictionary nlua_pop_Dictionary_unchecked(lua_State *lstate, const LuaTabl
 /// Convert lua table to dictionary
 ///
 /// Always pops one value from the stack.
-Dictionary nlua_pop_Dictionary(lua_State *lstate, bool ref, Error *err)
-  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+Dictionary nlua_pop_Dictionary(lua_State *lstate, bool ref, Arena *arena, Error *err)
+  FUNC_ATTR_NONNULL_ARG(1, 4) FUNC_ATTR_WARN_UNUSED_RESULT
 {
   const LuaTableProps table_props = nlua_check_type(lstate, err,
                                                     kObjectTypeDictionary);
@@ -1052,7 +1055,7 @@ Dictionary nlua_pop_Dictionary(lua_State *lstate, bool ref, Error *err)
     return (Dictionary) { .size = 0, .items = NULL };
   }
 
-  return nlua_pop_Dictionary_unchecked(lstate, table_props, ref, err);
+  return nlua_pop_Dictionary_unchecked(lstate, table_props, ref, arena, err);
 }
 
 /// Helper structure for nlua_pop_Object
@@ -1064,7 +1067,8 @@ typedef struct {
 /// Convert lua table to object
 ///
 /// Always pops one value from the stack.
-Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
+Object nlua_pop_Object(lua_State *const lstate, bool ref, Arena *arena, Error *const err)
+  FUNC_ATTR_NONNULL_ARG(1, 4) FUNC_ATTR_WARN_UNUSED_RESULT
 {
   Object ret = NIL;
   const int initial_size = lua_gettop(lstate);
@@ -1099,10 +1103,7 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
           size_t len;
           const char *s = lua_tolstring(lstate, -2, &len);
           const size_t idx = cur.obj->data.dictionary.size++;
-          cur.obj->data.dictionary.items[idx].key = (String) {
-            .data = xmemdupz(s, len),
-            .size = len,
-          };
+          cur.obj->data.dictionary.items[idx].key = CBUF_TO_ARENA_STR(arena, s, len);
           kvi_push(stack, cur);
           cur = (ObjPopStackItem){ .obj = &cur.obj->data.dictionary.items[idx].value };
         } else {
@@ -1133,7 +1134,7 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
     case LUA_TSTRING: {
       size_t len;
       const char *s = lua_tolstring(lstate, -1, &len);
-      *cur.obj = STRING_OBJ(((String) { .data = xmemdupz(s, len), .size = len }));
+      *cur.obj = STRING_OBJ(CBUF_TO_ARENA_STR(arena, s, len));
       break;
     }
     case LUA_TNUMBER: {
@@ -1151,23 +1152,17 @@ Object nlua_pop_Object(lua_State *const lstate, bool ref, Error *const err)
 
       switch (table_props.type) {
       case kObjectTypeArray:
-        *cur.obj = ARRAY_OBJ(((Array) { .items = NULL, .size = 0, .capacity = 0 }));
+        *cur.obj = ARRAY_OBJ(((Array)ARRAY_DICT_INIT));
         if (table_props.maxidx != 0) {
-          cur.obj->data.array.items =
-            xcalloc(table_props.maxidx,
-                    sizeof(cur.obj->data.array.items[0]));
-          cur.obj->data.array.capacity = table_props.maxidx;
+          cur.obj->data.array = arena_array(arena, table_props.maxidx);
           cur.container = true;
           kvi_push(stack, cur);
         }
         break;
       case kObjectTypeDictionary:
-        *cur.obj = DICTIONARY_OBJ(((Dictionary) { .items = NULL, .size = 0, .capacity = 0 }));
+        *cur.obj = DICTIONARY_OBJ(((Dictionary)ARRAY_DICT_INIT));
         if (table_props.string_keys_num != 0) {
-          cur.obj->data.dictionary.items =
-            xcalloc(table_props.string_keys_num,
-                    sizeof(cur.obj->data.dictionary.items[0]));
-          cur.obj->data.dictionary.capacity = table_props.string_keys_num;
+          cur.obj->data.dictionary = arena_dict(arena, table_props.string_keys_num);
           cur.container = true;
           kvi_push(stack, cur);
           lua_pushnil(lstate);
@@ -1219,7 +1214,9 @@ type_error:
   }
   kvi_destroy(stack);
   if (ERROR_SET(err)) {
-    api_free_object(ret);
+    if (!arena) {
+      api_free_object(ret);
+    }
     ret = NIL;
     lua_pop(lstate, lua_gettop(lstate) - initial_size + 1);
   }
@@ -1227,14 +1224,14 @@ type_error:
   return ret;
 }
 
-LuaRef nlua_pop_LuaRef(lua_State *const lstate, Error *err)
+LuaRef nlua_pop_LuaRef(lua_State *const lstate, Arena *arena, Error *err)
 {
   LuaRef rv = nlua_ref_global(lstate, -1);
   lua_pop(lstate, 1);
   return rv;
 }
 
-handle_T nlua_pop_handle(lua_State *lstate, Error *err)
+handle_T nlua_pop_handle(lua_State *lstate, Arena *arena, Error *err)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
   handle_T ret;
@@ -1296,7 +1293,8 @@ void nlua_init_types(lua_State *const lstate)
 }
 
 // lua specific variant of api_dict_to_keydict
-void nlua_pop_keydict(lua_State *L, void *retval, FieldHashfn hashy, char **err_opt, Error *err)
+void nlua_pop_keydict(lua_State *L, void *retval, FieldHashfn hashy, char **err_opt, Arena *arena,
+                      Error *err)
 {
   if (!lua_istable(L, -1)) {
     api_set_error(err, kErrorTypeValidation, "Expected Lua table");
@@ -1323,7 +1321,7 @@ void nlua_pop_keydict(lua_State *L, void *retval, FieldHashfn hashy, char **err_
     char *mem = ((char *)retval + field->ptr_off);
 
     if (field->type == kObjectTypeNil) {
-      *(Object *)mem = nlua_pop_Object(L, true, err);
+      *(Object *)mem = nlua_pop_Object(L, true, arena, err);
     } else if (field->type == kObjectTypeInteger) {
       if (field->is_hlgroup && lua_type(L, -1) == LUA_TSTRING) {
         size_t name_len;
@@ -1331,23 +1329,23 @@ void nlua_pop_keydict(lua_State *L, void *retval, FieldHashfn hashy, char **err_
         lua_pop(L, 1);
         *(Integer *)mem = name_len > 0 ? syn_check_group(name, name_len) : 0;
       } else {
-        *(Integer *)mem = nlua_pop_Integer(L, err);
+        *(Integer *)mem = nlua_pop_Integer(L, arena, err);
       }
     } else if (field->type == kObjectTypeBoolean) {
       *(Boolean *)mem = nlua_pop_Boolean_strict(L, err);
     } else if (field->type == kObjectTypeString) {
-      *(String *)mem = nlua_pop_String(L, err);
+      *(String *)mem = nlua_pop_String(L, arena, err);
     } else if (field->type == kObjectTypeFloat) {
-      *(Float *)mem = nlua_pop_Float(L, err);
+      *(Float *)mem = nlua_pop_Float(L, arena, err);
     } else if (field->type == kObjectTypeBuffer || field->type == kObjectTypeWindow
                || field->type == kObjectTypeTabpage) {
-      *(handle_T *)mem = nlua_pop_handle(L, err);
+      *(handle_T *)mem = nlua_pop_handle(L, arena, err);
     } else if (field->type == kObjectTypeArray) {
-      *(Array *)mem = nlua_pop_Array(L, err);
+      *(Array *)mem = nlua_pop_Array(L, arena, err);
     } else if (field->type == kObjectTypeDictionary) {
-      *(Dictionary *)mem = nlua_pop_Dictionary(L, false, err);
+      *(Dictionary *)mem = nlua_pop_Dictionary(L, false, arena, err);
     } else if (field->type == kObjectTypeLuaRef) {
-      *(LuaRef *)mem = nlua_pop_LuaRef(L, err);
+      *(LuaRef *)mem = nlua_pop_LuaRef(L, arena, err);
     } else {
       abort();
     }
diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c
index 85d614fe0d..be55bde202 100644
--- a/src/nvim/lua/executor.c
+++ b/src/nvim/lua/executor.c
@@ -311,7 +311,10 @@ static int nlua_thr_api_nvim__get_runtime(lua_State *lstate)
   lua_pop(lstate, 1);
 
   Error err = ERROR_INIT;
-  const Array pat = nlua_pop_Array(lstate, &err);
+  // TODO(bfredl): we could use an arena here for both "pat" and "ret", but then
+  // we need a path to not use the freelist but a private block local to the thread.
+  // We do not want mutex contentionery for the main arena freelist.
+  const Array pat = nlua_pop_Array(lstate, NULL, &err);
   if (ERROR_SET(&err)) {
     luaL_where(lstate, 1);
     lua_pushstring(lstate, err.msg);
@@ -1242,13 +1245,13 @@ static int nlua_rpc(lua_State *lstate, bool request)
   const char *name = luaL_checklstring(lstate, 2, &name_len);
   int nargs = lua_gettop(lstate) - 2;
   Error err = ERROR_INIT;
-  Array args = ARRAY_DICT_INIT;
+  Arena arena = ARENA_EMPTY;
 
+  Array args = arena_array(&arena, (size_t)nargs);
   for (int i = 0; i < nargs; i++) {
     lua_pushvalue(lstate, i + 3);
-    ADD(args, nlua_pop_Object(lstate, false, &err));
+    ADD(args, nlua_pop_Object(lstate, false, &arena, &err));
     if (ERROR_SET(&err)) {
-      api_free_array(args);
       goto check_err;
     }
   }
@@ -1265,10 +1268,11 @@ static int nlua_rpc(lua_State *lstate, bool request)
       api_set_error(&err, kErrorTypeValidation,
                     "Invalid channel: %" PRIu64, chan_id);
     }
-    api_free_array(args);  // TODO(bfredl): no
   }
 
 check_err:
+  arena_mem_free(arena_finish(&arena));
+
   if (ERROR_SET(&err)) {
     lua_pushstring(lstate, err.msg);
     api_clear_error(&err);
@@ -1541,10 +1545,12 @@ int typval_exec_lua_callable(LuaRef lua_cb, int argcount, typval_T *argvars, typ
 ///
 /// @param[in]  str  String to execute.
 /// @param[in]  args array of ... args
+/// @param[in]  mode Whether and how the the return value should be converted to Object
+/// @param[in] arena  can be NULL, then nested allocations are used
 /// @param[out]  err  Location where error will be saved.
 ///
 /// @return Return value of the execution.
-Object nlua_exec(const String str, const Array args, Error *err)
+Object nlua_exec(const String str, const Array args, LuaRetMode mode, Arena *arena, Error *err)
 {
   lua_State *const lstate = global_lstate;
 
@@ -1568,7 +1574,7 @@ Object nlua_exec(const String str, const Array args, Error *err)
     return NIL;
   }
 
-  return nlua_pop_Object(lstate, false, err);
+  return nlua_call_pop_retval(lstate, mode, arena, err);
 }
 
 bool nlua_ref_is_function(LuaRef ref)
@@ -1589,12 +1595,12 @@ bool nlua_ref_is_function(LuaRef ref)
 /// @param ref     the reference to call (not consumed)
 /// @param name    if non-NULL, sent to callback as first arg
 ///                if NULL, only args are used
-/// @param retval  if true, convert return value to Object
-///                if false, only check if return value is truthy
+/// @param mode    Whether and how the the return value should be converted to Object
+/// @param arena   can be NULL, then nested allocations are used
 /// @param err     Error details, if any (if NULL, errors are echoed)
-/// @return        Return value of function, if retval was set. Otherwise
-/// BOOLEAN_OBJ(true) or NIL.
-Object nlua_call_ref(LuaRef ref, const char *name, Array args, bool retval, Error *err)
+/// @return        Return value of function, as per mode
+Object nlua_call_ref(LuaRef ref, const char *name, Array args, LuaRetMode mode, Arena *arena,
+                     Error *err)
 {
   lua_State *const lstate = global_lstate;
   nlua_pushref(lstate, ref);
@@ -1620,18 +1626,34 @@ Object nlua_call_ref(LuaRef ref, const char *name, Array args, bool retval, Erro
     return NIL;
   }
 
-  if (retval) {
-    Error dummy = ERROR_INIT;
-    if (err == NULL) {
-      err = &dummy;
-    }
-    return nlua_pop_Object(lstate, false, err);
-  } else {
-    bool value = lua_toboolean(lstate, -1);
+  return nlua_call_pop_retval(lstate, mode, arena, err);
+}
+
+static Object nlua_call_pop_retval(lua_State *lstate, LuaRetMode mode, Arena *arena, Error *err)
+{
+  if (lua_isnil(lstate, -1)) {
+    lua_pop(lstate, 1);
+    return NIL;
+  }
+  Error dummy = ERROR_INIT;
+
+  switch (mode) {
+  case kRetNilBool: {
+    bool bool_value = lua_toboolean(lstate, -1);
+    lua_pop(lstate, 1);
+
+    return BOOLEAN_OBJ(bool_value);
+  }
+  case kRetLuaref: {
+    LuaRef ref = nlua_ref_global(lstate, -1);
     lua_pop(lstate, 1);
 
-    return value ? BOOLEAN_OBJ(true) : NIL;
+    return LUAREF_OBJ(ref);
+  }
+  case kRetObject:
+    return nlua_pop_Object(lstate, false, arena, err ? err : &dummy);
   }
+  UNREACHABLE;
 }
 
 /// check if the current execution context is safe for calling deferred API
@@ -1930,13 +1952,14 @@ int nlua_expand_pat(expand_T *xp, char *pat, int *num_results, char ***results)
   *num_results = 0;
   *results = NULL;
 
-  int prefix_len = (int)nlua_pop_Integer(lstate, &err);
+  Arena arena = ARENA_EMPTY;
+  int prefix_len = (int)nlua_pop_Integer(lstate, &arena, &err);
   if (ERROR_SET(&err)) {
     ret = FAIL;
     goto cleanup;
   }
 
-  Array completions = nlua_pop_Array(lstate, &err);
+  Array completions = nlua_pop_Array(lstate, &arena, &err);
   if (ERROR_SET(&err)) {
     ret = FAIL;
     goto cleanup_array;
@@ -1960,7 +1983,7 @@ int nlua_expand_pat(expand_T *xp, char *pat, int *num_results, char ***results)
   *num_results = result_array.ga_len;
 
 cleanup_array:
-  api_free_array(completions);
+  arena_mem_free(arena_finish(&arena));
 
 cleanup:
 
@@ -2354,13 +2377,10 @@ bool nlua_func_exists(const char *lua_funcname)
   vim_snprintf(str, length, "return %s", lua_funcname);
   ADD_C(args, CSTR_AS_OBJ(str));
   Error err = ERROR_INIT;
-  Object result = NLUA_EXEC_STATIC("return type(loadstring(...)()) == 'function'", args, &err);
+  Object result = NLUA_EXEC_STATIC("return type(loadstring(...)()) == 'function'", args,
+                                   kRetNilBool, NULL, &err);
   xfree(str);
 
   api_clear_error(&err);
-  if (result.type != kObjectTypeBoolean) {
-    api_free_object(result);
-    return false;
-  }
-  return result.data.boolean;
+  return LUARET_TRUTHY(result);
 }
diff --git a/src/nvim/lua/executor.h b/src/nvim/lua/executor.h
index 0b4623cbd3..ebcd62122f 100644
--- a/src/nvim/lua/executor.h
+++ b/src/nvim/lua/executor.h
@@ -24,7 +24,8 @@ typedef struct {
 #endif
 } nlua_ref_state_t;
 
-#define NLUA_EXEC_STATIC(cstr, arg, err) nlua_exec(STATIC_CSTR_AS_STRING(cstr), arg, err)
+#define NLUA_EXEC_STATIC(cstr, arg, mode, arena, err) \
+  nlua_exec(STATIC_CSTR_AS_STRING(cstr), arg, mode, arena, err)
 
 #define NLUA_CLEAR_REF(x) \
   do { \
@@ -35,6 +36,16 @@ typedef struct {
     } \
   } while (0)
 
+typedef enum {
+  kRetObject,  ///< any object, but doesn't preserve nested luarefs
+  kRetNilBool,  ///< NIL preserved as such, other values return their booleanness
+                ///< Should also be used when return value is ignored, as it is allocation-free
+  kRetLuaref,  ///< return value becomes a single Luaref, regardless of type (except NIL)
+} LuaRetMode;
+
+/// To use with kRetNilBool for quick thuthyness check
+#define LUARET_TRUTHY(res) ((res).type == kObjectTypeBoolean && (res).data.boolean == true)
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "lua/executor.h.generated.h"
 #endif
diff --git a/src/nvim/lua/xdiff.c b/src/nvim/lua/xdiff.c
index 5285c1187d..035c171a14 100644
--- a/src/nvim/lua/xdiff.c
+++ b/src/nvim/lua/xdiff.c
@@ -5,7 +5,9 @@
 #include <string.h>
 
 #include "luaconf.h"
+#include "nvim/api/keysets_defs.h"
 #include "nvim/api/private/defs.h"
+#include "nvim/api/private/dispatch.h"
 #include "nvim/api/private/helpers.h"
 #include "nvim/linematch.h"
 #include "nvim/lua/converter.h"
@@ -187,136 +189,84 @@ static mmfile_t get_string_arg(lua_State *lstate, int idx)
   return mf;
 }
 
-// Helper function for validating option types
-static bool check_xdiff_opt(ObjectType actType, ObjectType expType, const char *name, Error *err)
-{
-  if (actType != expType) {
-    const char *type_str =
-      expType == kObjectTypeString
-      ? "string" : (expType == kObjectTypeInteger
-                    ? "integer" : (expType == kObjectTypeBoolean
-                                   ? "boolean" : (expType == kObjectTypeLuaRef
-                                                  ? "function" : "NA")));
-
-    api_set_error(err, kErrorTypeValidation, "%s is not a %s", name,
-                  type_str);
-    return true;
-  }
-
-  return false;
-}
-
 static NluaXdiffMode process_xdl_diff_opts(lua_State *lstate, xdemitconf_t *cfg, xpparam_t *params,
                                            int64_t *linematch, Error *err)
 {
-  const DictionaryOf(LuaRef) opts = nlua_pop_Dictionary(lstate, true, err);
+  Dict(xdl_diff) opts = KEYDICT_INIT;
+  char *err_param = NULL;
+  KeySetLink *KeyDict_xdl_diff_get_field(const char *str, size_t len);
+  nlua_pop_keydict(lstate, &opts, KeyDict_xdl_diff_get_field, &err_param, NULL, err);
 
   NluaXdiffMode mode = kNluaXdiffModeUnified;
 
-  bool had_on_hunk = false;
   bool had_result_type_indices = false;
-  for (size_t i = 0; i < opts.size; i++) {
-    String k = opts.items[i].key;
-    Object *v = &opts.items[i].value;
-    if (strequal("on_hunk", k.data)) {
-      if (check_xdiff_opt(v->type, kObjectTypeLuaRef, "on_hunk", err)) {
-        goto exit_1;
-      }
-      had_on_hunk = true;
-      nlua_pushref(lstate, v->data.luaref);
-    } else if (strequal("result_type", k.data)) {
-      if (check_xdiff_opt(v->type, kObjectTypeString, "result_type", err)) {
-        goto exit_1;
-      }
-      if (strequal("unified", v->data.string.data)) {
-        // the default
-      } else if (strequal("indices", v->data.string.data)) {
-        had_result_type_indices = true;
-      } else {
-        api_set_error(err, kErrorTypeValidation, "not a valid result_type");
-        goto exit_1;
-      }
-    } else if (strequal("algorithm", k.data)) {
-      if (check_xdiff_opt(v->type, kObjectTypeString, "algorithm", err)) {
-        goto exit_1;
-      }
-      if (strequal("myers", v->data.string.data)) {
-        // default
-      } else if (strequal("minimal", v->data.string.data)) {
-        params->flags |= XDF_NEED_MINIMAL;
-      } else if (strequal("patience", v->data.string.data)) {
-        params->flags |= XDF_PATIENCE_DIFF;
-      } else if (strequal("histogram", v->data.string.data)) {
-        params->flags |= XDF_HISTOGRAM_DIFF;
-      } else {
-        api_set_error(err, kErrorTypeValidation, "not a valid algorithm");
-        goto exit_1;
-      }
-    } else if (strequal("ctxlen", k.data)) {
-      if (check_xdiff_opt(v->type, kObjectTypeInteger, "ctxlen", err)) {
-        goto exit_1;
-      }
-      cfg->ctxlen = (long)v->data.integer;
-    } else if (strequal("interhunkctxlen", k.data)) {
-      if (check_xdiff_opt(v->type, kObjectTypeInteger, "interhunkctxlen",
-                          err)) {
-        goto exit_1;
-      }
-      cfg->interhunkctxlen = (long)v->data.integer;
-    } else if (strequal("linematch", k.data)) {
-      if (v->type == kObjectTypeBoolean) {
-        *linematch = v->data.boolean ? INT64_MAX : 0;
-      } else if (v->type == kObjectTypeInteger) {
-        *linematch = v->data.integer;
-      } else {
-        api_set_error(err, kErrorTypeValidation, "linematch must be a boolean or integer");
-        goto exit_1;
-      }
+
+  if (HAS_KEY(&opts, xdl_diff, result_type)) {
+    if (strequal("unified", opts.result_type.data)) {
+      // the default
+    } else if (strequal("indices", opts.result_type.data)) {
+      had_result_type_indices = true;
     } else {
-      struct {
-        const char *name;
-        unsigned long value;
-      } flags[] = {
-        { "ignore_whitespace", XDF_IGNORE_WHITESPACE },
-        { "ignore_whitespace_change", XDF_IGNORE_WHITESPACE_CHANGE },
-        { "ignore_whitespace_change_at_eol", XDF_IGNORE_WHITESPACE_AT_EOL },
-        { "ignore_cr_at_eol", XDF_IGNORE_CR_AT_EOL },
-        { "ignore_blank_lines", XDF_IGNORE_BLANK_LINES },
-        { "indent_heuristic", XDF_INDENT_HEURISTIC },
-        {  NULL, 0 },
-      };
-      bool key_used = false;
-      for (size_t j = 0; flags[j].name; j++) {
-        if (strequal(flags[j].name, k.data)) {
-          if (check_xdiff_opt(v->type, kObjectTypeBoolean, flags[j].name,
-                              err)) {
-            goto exit_1;
-          }
-          if (v->data.boolean) {
-            params->flags |= flags[j].value;
-          }
-          key_used = true;
-          break;
-        }
-      }
+      api_set_error(err, kErrorTypeValidation, "not a valid result_type");
+      goto exit_1;
+    }
+  }
 
-      if (key_used) {
-        continue;
-      }
+  if (HAS_KEY(&opts, xdl_diff, algorithm)) {
+    if (strequal("myers", opts.algorithm.data)) {
+      // default
+    } else if (strequal("minimal", opts.algorithm.data)) {
+      params->flags |= XDF_NEED_MINIMAL;
+    } else if (strequal("patience", opts.algorithm.data)) {
+      params->flags |= XDF_PATIENCE_DIFF;
+    } else if (strequal("histogram", opts.algorithm.data)) {
+      params->flags |= XDF_HISTOGRAM_DIFF;
+    } else {
+      api_set_error(err, kErrorTypeValidation, "not a valid algorithm");
+      goto exit_1;
+    }
+  }
 
-      api_set_error(err, kErrorTypeValidation, "unexpected key: %s", k.data);
+  if (HAS_KEY(&opts, xdl_diff, ctxlen)) {
+    cfg->ctxlen = (long)opts.ctxlen;
+  }
+
+  if (HAS_KEY(&opts, xdl_diff, interhunkctxlen)) {
+    cfg->interhunkctxlen = (long)opts.interhunkctxlen;
+  }
+
+  if (HAS_KEY(&opts, xdl_diff, linematch)) {
+    if (opts.linematch.type == kObjectTypeBoolean) {
+      *linematch = opts.linematch.data.boolean ? INT64_MAX : 0;
+    } else if (opts.linematch.type == kObjectTypeInteger) {
+      *linematch = opts.linematch.data.integer;
+    } else {
+      api_set_error(err, kErrorTypeValidation, "linematch must be a boolean or integer");
       goto exit_1;
     }
   }
 
-  if (had_on_hunk) {
+  params->flags |= opts.ignore_whitespace ? XDF_IGNORE_WHITESPACE : 0;
+  params->flags |= opts.ignore_whitespace_change ? XDF_IGNORE_WHITESPACE_CHANGE : 0;
+  params->flags |= opts.ignore_whitespace_change_at_eol ? XDF_IGNORE_WHITESPACE_AT_EOL : 0;
+  params->flags |= opts.ignore_cr_at_eol ? XDF_IGNORE_CR_AT_EOL : 0;
+  params->flags |= opts.ignore_blank_lines ? XDF_IGNORE_BLANK_LINES : 0;
+  params->flags |= opts.indent_heuristic ? XDF_INDENT_HEURISTIC : 0;
+
+  if (HAS_KEY(&opts, xdl_diff, on_hunk)) {
     mode = kNluaXdiffModeOnHunkCB;
+    nlua_pushref(lstate, opts.on_hunk);
+    if (lua_type(lstate, -1) != LUA_TFUNCTION) {
+      api_set_error(err, kErrorTypeValidation, "on_hunk is not a function");
+    }
   } else if (had_result_type_indices) {
     mode = kNluaXdiffModeLocations;
   }
 
 exit_1:
-  api_free_dictionary(opts);
+  api_free_string(opts.result_type);
+  api_free_string(opts.algorithm);
+  api_free_luaref(opts.on_hunk);
   return mode;
 }