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).

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;

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_SINCE(7) FUNC_API_DEPRECATED_SINCE(11)
-  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_SINCE(1) FUNC_API_DEPRECATED_SINCE(7)
-  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)
 {

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) {

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;

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)
+  bool ret_alloc;  ///< return value is allocated and should be freed using api_free_object
-                      ///< and should not be freed as such.
+                   ///< otherwise it uses arena and/or static memory
 };
 
 extern const MsgpackRpcRequestHandler method_handlers[];

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);

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.

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;
 

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));

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.

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)

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.receives_arena = true
-      fn.arena_return = 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 = '
+      .. ', .ret_alloc = '
-      .. tostring(not not fn.arena_return)
+      .. 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)

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);
   }