refactor(api): make freeing of return-value opt-in instead of opt out

As only a few API functions make use of explicit freeing of the return
value, make it opt-in instead. The arena is always present under the
hood, so `Arena *arena` arg now doesn't mean anything other than getting
access to this arena. Also it is in principle possible to return an
allocated value while still using the arena as scratch space for other
stuff (unlikely, but there no reason to not allow it).

13 files changed, 33 insertions, 37 deletions

diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c
index ad25451054..41a3743895 100644
--- a/src/nvim/api/buffer.c
+++ b/src/nvim/api/buffer.c
@@ -956,7 +956,7 @@ void nvim_buf_del_var(Buffer buffer, String name, Error *err)
 /// @param buffer     Buffer handle, or 0 for current buffer
 /// @param[out] err   Error details, if any
 /// @return Buffer name
-String nvim_buf_get_name(Buffer buffer, Arena *arena, Error *err)
+String nvim_buf_get_name(Buffer buffer, Error *err)
   FUNC_API_SINCE(1)
 {
   String rv = STRING_INIT;
diff --git a/src/nvim/api/deprecated.c b/src/nvim/api/deprecated.c
index f805c945f3..404e85cf91 100644
--- a/src/nvim/api/deprecated.c
+++ b/src/nvim/api/deprecated.c
@@ -32,8 +32,8 @@
 /// @deprecated Use nvim_exec2() instead.
 /// @see nvim_exec2
 String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)
-  FUNC_API_SINCE(7)
-  FUNC_API_DEPRECATED_SINCE(11)
+  FUNC_API_SINCE(7) FUNC_API_DEPRECATED_SINCE(11)
+  FUNC_API_RET_ALLOC
 {
   Dict(exec_opts) opts = { .output = output };
   return exec_impl(channel_id, src, &opts, err);
@@ -42,8 +42,8 @@ String nvim_exec(uint64_t channel_id, String src, Boolean output, Error *err)
 /// @deprecated
 /// @see nvim_exec2
 String nvim_command_output(uint64_t channel_id, String command, Error *err)
-  FUNC_API_SINCE(1)
-  FUNC_API_DEPRECATED_SINCE(7)
+  FUNC_API_SINCE(1) FUNC_API_DEPRECATED_SINCE(7)
+  FUNC_API_RET_ALLOC
 {
   Dict(exec_opts) opts = { .output = true };
   return exec_impl(channel_id, command, &opts, err);
@@ -541,7 +541,7 @@ void nvim_set_option(uint64_t channel_id, String name, Object value, Error *err)
 /// @param name     Option name
 /// @param[out] err Error details, if any
 /// @return         Option value (global)
-Object nvim_get_option(String name, Arena *arena, Error *err)
+Object nvim_get_option(String name, Error *err)
   FUNC_API_SINCE(1)
   FUNC_API_DEPRECATED_SINCE(11)
 {
@@ -555,7 +555,7 @@ Object nvim_get_option(String name, Arena *arena, Error *err)
 /// @param name       Option name
 /// @param[out] err   Error details, if any
 /// @return Option value
-Object nvim_buf_get_option(Buffer buffer, String name, Arena *arena, Error *err)
+Object nvim_buf_get_option(Buffer buffer, String name, Error *err)
   FUNC_API_SINCE(1)
   FUNC_API_DEPRECATED_SINCE(11)
 {
@@ -597,7 +597,7 @@ void nvim_buf_set_option(uint64_t channel_id, Buffer buffer, String name, Object
 /// @param name     Option name
 /// @param[out] err Error details, if any
 /// @return Option value
-Object nvim_win_get_option(Window window, String name, Arena *arena, Error *err)
+Object nvim_win_get_option(Window window, String name, Error *err)
   FUNC_API_SINCE(1)
   FUNC_API_DEPRECATED_SINCE(11)
 {
diff --git a/src/nvim/api/extmark.c b/src/nvim/api/extmark.c
index 0b5f3471fb..fb729be23e 100644
--- a/src/nvim/api/extmark.c
+++ b/src/nvim/api/extmark.c
@@ -1203,7 +1203,7 @@ free_exit:
 }
 
 String nvim__buf_debug_extmarks(Buffer buffer, Boolean keys, Boolean dot, Error *err)
-  FUNC_API_SINCE(7)
+  FUNC_API_SINCE(7) FUNC_API_RET_ALLOC
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
   if (!buf) {
diff --git a/src/nvim/api/options.c b/src/nvim/api/options.c
index 8128fdf67b..7deadb8eb5 100644
--- a/src/nvim/api/options.c
+++ b/src/nvim/api/options.c
@@ -150,7 +150,7 @@ static buf_T *do_ft_buf(char *filetype, aco_save_T *aco, Error *err)
 /// @param[out] err  Error details, if any
 /// @return          Option value
 Object nvim_get_option_value(String name, Dict(option) *opts, Error *err)
-  FUNC_API_SINCE(9)
+  FUNC_API_SINCE(9) FUNC_API_RET_ALLOC
 {
   OptIndex opt_idx = 0;
   int scope = 0;
diff --git a/src/nvim/api/private/dispatch.h b/src/nvim/api/private/dispatch.h
index 6d051176ac..288f368fba 100644
--- a/src/nvim/api/private/dispatch.h
+++ b/src/nvim/api/private/dispatch.h
@@ -18,8 +18,8 @@ struct MsgpackRpcRequestHandler {
               ///< uv loop (the loop is run very frequently due to breakcheck).
               ///< If "fast" is false, the function is deferred, i e the call will
               ///< be put in the event queue, for safe handling later.
-  bool arena_return;  ///< return value is allocated in the arena (or statically)
-                      ///< and should not be freed as such.
+  bool ret_alloc;  ///< return value is allocated and should be freed using api_free_object
+                   ///< otherwise it uses arena and/or static memory
 };
 
 extern const MsgpackRpcRequestHandler method_handlers[];
diff --git a/src/nvim/api/private/helpers.h b/src/nvim/api/private/helpers.h
index 20bc889a0a..7eda8ffaf6 100644
--- a/src/nvim/api/private/helpers.h
+++ b/src/nvim/api/private/helpers.h
@@ -125,13 +125,6 @@ typedef kvec_withinit_t(Object, 16) ArrayBuilder;
 
 #define KEYDICT_INIT { 0 }
 
-#define api_free_boolean(value)
-#define api_free_integer(value)
-#define api_free_float(value)
-#define api_free_buffer(value)
-#define api_free_window(value)
-#define api_free_tabpage(value)
-
 EXTERN PMap(int) buffer_handles INIT( = MAP_INIT);
 EXTERN PMap(int) window_handles INIT( = MAP_INIT);
 EXTERN PMap(int) tabpage_handles INIT( = MAP_INIT);
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index c6043a2871..2888656a7a 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -461,7 +461,7 @@ error:
 /// @see replace_termcodes
 /// @see cpoptions
 String nvim_replace_termcodes(String str, Boolean from_part, Boolean do_lt, Boolean special)
-  FUNC_API_SINCE(1)
+  FUNC_API_SINCE(1) FUNC_API_RET_ALLOC
 {
   if (str.size == 0) {
     // Empty string
@@ -601,6 +601,7 @@ static bool find_runtime_cb(int num_fnames, char **fnames, bool all, void *c)
 }
 
 String nvim__get_lib_dir(void)
+  FUNC_API_RET_ALLOC
 {
   return cstr_as_string(get_lib_dir());
 }
@@ -1726,7 +1727,7 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
     // directly here. But `result` might become invalid when next api function
     // is called in the loop.
     ADD_C(results, copy_object(result, arena));
-    if (!handler.arena_return) {
+    if (handler.ret_alloc) {
       api_free_object(result);
     }
   }
@@ -1806,9 +1807,9 @@ static void write_msg(String message, bool to_err, bool writeln)
 /// @param[in]  obj  Object to return.
 ///
 /// @return its argument.
-Object nvim__id(Object obj)
+Object nvim__id(Object obj, Arena *arena)
 {
-  return copy_object(obj, NULL);
+  return copy_object(obj, arena);
 }
 
 /// Returns array given as argument.
@@ -1819,9 +1820,9 @@ Object nvim__id(Object obj)
 /// @param[in]  arr  Array to return.
 ///
 /// @return its argument.
-Array nvim__id_array(Array arr)
+Array nvim__id_array(Array arr, Arena *arena)
 {
-  return copy_array(arr, NULL);
+  return copy_array(arr, arena);
 }
 
 /// Returns dictionary given as argument.
@@ -1832,9 +1833,9 @@ Array nvim__id_array(Array arr)
 /// @param[in]  dct  Dictionary to return.
 ///
 /// @return its argument.
-Dictionary nvim__id_dictionary(Dictionary dct)
+Dictionary nvim__id_dictionary(Dictionary dct, Arena *arena)
 {
-  return copy_dictionary(dct, NULL);
+  return copy_dictionary(dct, arena);
 }
 
 /// Returns floating-point value given as argument.
diff --git a/src/nvim/api/vimscript.c b/src/nvim/api/vimscript.c
index db32eac3d1..7239971c21 100644
--- a/src/nvim/api/vimscript.c
+++ b/src/nvim/api/vimscript.c
@@ -51,7 +51,7 @@
 /// @return Dictionary containing information about execution, with these keys:
 ///       - output: (string|nil) Output if `opts.output` is true.
 Dictionary nvim_exec2(uint64_t channel_id, String src, Dict(exec_opts) *opts, Error *err)
-  FUNC_API_SINCE(11)
+  FUNC_API_SINCE(11) FUNC_API_RET_ALLOC
 {
   Dictionary result = ARRAY_DICT_INIT;
 
diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
index e5af50c8f8..01e666887a 100644
--- a/src/nvim/eval/funcs.c
+++ b/src/nvim/eval/funcs.c
@@ -377,7 +377,7 @@ static void api_wrapper(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   object_to_vim_take_luaref(&result, rettv, true, &err);
 
 end:
-  if (!handler.arena_return) {
+  if (handler.ret_alloc) {
     api_free_object(result);
   }
   arena_mem_free(arena_finish(&arena));
diff --git a/src/nvim/func_attr.h b/src/nvim/func_attr.h
index d638fda531..43af880767 100644
--- a/src/nvim/func_attr.h
+++ b/src/nvim/func_attr.h
@@ -212,6 +212,8 @@
 #if defined(DEFINE_FUNC_ATTRIBUTES) || defined(DEFINE_EMPTY_ATTRIBUTES)
 /// Fast (non-deferred) API function.
 # define FUNC_API_FAST
+/// Return value needs to be freed
+# define FUNC_API_RET_ALLOC
 /// Internal C function not exposed in the RPC API.
 # define FUNC_API_NOEXPORT
 /// API function not exposed in Vimscript/eval.
diff --git a/src/nvim/generators/c_grammar.lua b/src/nvim/generators/c_grammar.lua
index 1720b32919..8a3e70990a 100644
--- a/src/nvim/generators/c_grammar.lua
+++ b/src/nvim/generators/c_grammar.lua
@@ -47,6 +47,7 @@ local c_proto = Ct(
     * (fill * Cg((P('FUNC_API_SINCE(') * C(num ^ 1)) * P(')'), 'since') ^ -1)
     * (fill * Cg((P('FUNC_API_DEPRECATED_SINCE(') * C(num ^ 1)) * P(')'), 'deprecated_since') ^ -1)
     * (fill * Cg((P('FUNC_API_FAST') * Cc(true)), 'fast') ^ -1)
+    * (fill * Cg((P('FUNC_API_RET_ALLOC') * Cc(true)), 'ret_alloc') ^ -1)
     * (fill * Cg((P('FUNC_API_NOEXPORT') * Cc(true)), 'noexport') ^ -1)
     * (fill * Cg((P('FUNC_API_REMOTE_ONLY') * Cc(true)), 'remote_only') ^ -1)
     * (fill * Cg((P('FUNC_API_LUA_ONLY') * Cc(true)), 'lua_only') ^ -1)
diff --git a/src/nvim/generators/gen_api_dispatch.lua b/src/nvim/generators/gen_api_dispatch.lua
index 90ed90d2bc..c3fc5aa0a3 100644
--- a/src/nvim/generators/gen_api_dispatch.lua
+++ b/src/nvim/generators/gen_api_dispatch.lua
@@ -60,8 +60,7 @@ local function add_function(fn)
       fn.parameters[#fn.parameters] = nil
     end
     if #fn.parameters ~= 0 and fn.parameters[#fn.parameters][1] == 'arena' then
-      -- return value is allocated in an arena
-      fn.arena_return = true
+      fn.receives_arena = true
       fn.parameters[#fn.parameters] = nil
     end
   end
@@ -554,7 +553,7 @@ for i = 1, #functions do
       table.insert(call_args, a)
     end
 
-    if fn.arena_return then
+    if fn.receives_arena then
       table.insert(call_args, 'arena')
     end
 
@@ -621,8 +620,8 @@ for n, name in ipairs(hashorder) do
       .. (fn.impl_name or fn.name)
       .. ', .fast = '
       .. tostring(fn.fast)
-      .. ', .arena_return = '
-      .. tostring(not not fn.arena_return)
+      .. ', .ret_alloc = '
+      .. tostring(not not fn.ret_alloc)
       .. '},\n'
   )
 end
@@ -791,7 +790,7 @@ local function process_function(fn)
   if fn.receives_channel_id then
     cparams = 'LUA_INTERNAL_CALL, ' .. cparams
   end
-  if fn.arena_return then
+  if fn.receives_arena then
     cparams = cparams .. '&arena, '
   end
 
@@ -839,7 +838,7 @@ exit_0:
       return_type = fn.return_type
     end
     local free_retval = ''
-    if not fn.arena_return then
+    if fn.ret_alloc then
       free_retval = '  api_free_' .. return_type:lower() .. '(ret);'
     end
     write_shifted_output('    %s ret = %s(%s);\n', fn.return_type, fn.name, cparams)
diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c
index 1208c91760..ee7359c476 100644
--- a/src/nvim/msgpack_rpc/channel.c
+++ b/src/nvim/msgpack_rpc/channel.c
@@ -452,7 +452,7 @@ static void request_event(void **argv)
                                               &result,
                                               &out_buffer));
   }
-  if (!handler.arena_return) {
+  if (handler.ret_alloc) {
     api_free_object(result);
   }