refactor(autocmds): remove indirection #32291

Problem:
`AucmdExecutable` adds 2 layers of indirection. Although formalizing
a `union` is good practice for shared interfaces, this struct is mainly
for `autocmd_register` which is internal to this module.

Solution:
- Store the cmd/fn fields directly on the `AutoCmd` struct.
- Drop `AucmdExecutable` and related structures.

1 files changed, 42 insertions, 84 deletions

diff --git a/src/nvim/autocmd.c b/src/nvim/autocmd.c
index 6a7cf1c9a3..96da4695de 100644
--- a/src/nvim/autocmd.c
+++ b/src/nvim/autocmd.c
@@ -74,7 +74,6 @@ static const char e_autocommand_nesting_too_deep[]
 // Naming Conventions:
 //  - general autocmd behavior start with au_
 //  - AutoCmd start with aucmd_
-//  - Autocmd.exec stat with aucmd_exec
 //  - AutoPat start with aupat_
 //  - Groups start with augroup_
 //  - Events start with event_
@@ -254,24 +253,24 @@ static void au_show_for_event(int group, event_T event, const char *pat)
         return;
       }
 
-      char *exec_to_string = aucmd_exec_to_string(ac, ac->exec);
+      char *handler_str = aucmd_handler_to_string(ac);
       if (ac->desc != NULL) {
         size_t msglen = 100;
         char *msg = xmallocz(msglen);
-        if (ac->exec.type == CALLABLE_CB) {
-          msg_puts_hl(exec_to_string, HLF_8, false);
-          snprintf(msg, msglen, " [%s]", ac->desc);
+        if (ac->handler_cmd) {
+          snprintf(msg, msglen, "%s [%s]", handler_str, ac->desc);
         } else {
-          snprintf(msg, msglen, "%s [%s]", exec_to_string, ac->desc);
+          msg_puts_hl(handler_str, HLF_8, false);
+          snprintf(msg, msglen, " [%s]", ac->desc);
         }
         msg_outtrans(msg, 0, false);
         XFREE_CLEAR(msg);
-      } else if (ac->exec.type == CALLABLE_CB) {
-        msg_puts_hl(exec_to_string, HLF_8, false);
+      } else if (ac->handler_cmd) {
+        msg_outtrans(handler_str, 0, false);
       } else {
-        msg_outtrans(exec_to_string, 0, false);
+        msg_puts_hl(handler_str, HLF_8, false);
       }
-      XFREE_CLEAR(exec_to_string);
+      XFREE_CLEAR(handler_str);
       if (p_verbose > 0) {
         last_set_msg(ac->script_ctx);
       }
@@ -303,7 +302,11 @@ static void aucmd_del(AutoCmd *ac)
     xfree(ac->pat);
   }
   ac->pat = NULL;
-  aucmd_exec_free(&ac->exec);
+  if (ac->handler_cmd) {
+    XFREE_CLEAR(ac->handler_cmd);
+  } else {
+    callback_free(&ac->handler_fn);
+  }
   XFREE_CLEAR(ac->desc);
 
   au_need_clean = true;
@@ -899,8 +902,8 @@ void do_all_autocmd_events(const char *pat, bool once, int nested, char *cmd, bo
 // If *cmd == NUL: show entries.
 // If forceit == true: delete entries.
 // If group is not AUGROUP_ALL: only use this group.
-int do_autocmd_event(event_T event, const char *pat, bool once, int nested, char *cmd, bool del,
-                     int group)
+int do_autocmd_event(event_T event, const char *pat, bool once, int nested, const char *cmd,
+                     bool del, int group)
   FUNC_ATTR_NONNULL_ALL
 {
   // Cannot be used to show all patterns. See au_show_for_event or au_show_for_all_events
@@ -958,10 +961,8 @@ int do_autocmd_event(event_T event, const char *pat, bool once, int nested, char
     }
 
     if (is_adding_cmd) {
-      AucmdExecutable exec = AUCMD_EXECUTABLE_INIT;
-      exec.type = CALLABLE_EX;
-      exec.callable.cmd = cmd;
-      autocmd_register(0, event, pat, patlen, group, once, nested, NULL, exec);
+      Callback handler_fn = CALLBACK_INIT;
+      autocmd_register(0, event, pat, patlen, group, once, nested, NULL, cmd, &handler_fn);
     }
 
     pat = aucmd_next_pattern(pat, (size_t)patlen);
@@ -972,8 +973,13 @@ int do_autocmd_event(event_T event, const char *pat, bool once, int nested, char
   return OK;
 }
 
+/// Registers an autocmd. The handler may be a Ex command or callback function, decided by
+/// the `handler_cmd` or `handler_fn` args.
+///
+/// @param handler_cmd Handler Ex command, or NULL if handler is a function (`handler_fn`).
+/// @param handler_fn Handler function, ignored if `handler_cmd` is not NULL.
 int autocmd_register(int64_t id, event_T event, const char *pat, int patlen, int group, bool once,
-                     bool nested, char *desc, AucmdExecutable aucmd)
+                     bool nested, char *desc, const char *handler_cmd, Callback *handler_fn)
 {
   // 0 is not a valid group.
   assert(group != 0);
@@ -1081,7 +1087,12 @@ int autocmd_register(int64_t id, event_T event, const char *pat, int patlen, int
   AutoCmd *ac = kv_pushp(autocmds[(int)event]);
   ac->pat = ap;
   ac->id = id;
-  ac->exec = aucmd_exec_copy(aucmd);
+  if (handler_cmd) {
+    ac->handler_cmd = xstrdup(handler_cmd);
+  } else {
+    ac->handler_cmd = NULL;
+    callback_copy(&ac->handler_fn, handler_fn);
+  }
   ac->script_ctx = current_sctx;
   ac->script_ctx.sc_lnum += SOURCING_LNUM;
   nlua_set_sctx(&ac->script_ctx);
@@ -2028,7 +2039,7 @@ static void aucmd_next(AutoPatCmd *apc)
 /// Executes an autocmd callback function (as opposed to an Ex command).
 static bool au_callback(const AutoCmd *ac, const AutoPatCmd *apc)
 {
-  Callback callback = ac->exec.callable.cb;
+  Callback callback = ac->handler_fn;
   if (callback.type == kCallbackLua) {
     MAXSIZE_TEMP_DICT(data, 7);
     PUT_C(data, "id", INTEGER_OBJ(ac->id));
@@ -2093,10 +2104,10 @@ char *getnextac(int c, void *cookie, int indent, bool do_concat)
 
   if (p_verbose >= 9) {
     verbose_enter_scroll();
-    char *exec_to_string = aucmd_exec_to_string(ac, ac->exec);
-    smsg(0, _("autocommand %s"), exec_to_string);
+    char *handler_str = aucmd_handler_to_string(ac);
+    smsg(0, _("autocommand %s"), handler_str);
     msg_puts("\n");  // don't overwrite this either
-    XFREE_CLEAR(exec_to_string);
+    XFREE_CLEAR(handler_str);
     verbose_leave_scroll();
   }
 
@@ -2107,11 +2118,9 @@ char *getnextac(int c, void *cookie, int indent, bool do_concat)
   apc->script_ctx = current_sctx;
 
   char *retval;
-  switch (ac->exec.type) {
-  case CALLABLE_EX:
-    retval = xstrdup(ac->exec.callable.cmd);
-    break;
-  case CALLABLE_CB: {
+  if (ac->handler_cmd) {
+    retval = xstrdup(ac->handler_cmd);
+  } else {
     AutoCmd ac_copy = *ac;
     // Mark oneshot handler as "removed" now, to prevent recursion by e.g. `:doautocmd`. #25526
     ac->pat = oneshot ? NULL : ac->pat;
@@ -2133,11 +2142,6 @@ char *getnextac(int c, void *cookie, int indent, bool do_concat)
     //      and instead we loop over all the matches and just execute one-by-one.
     //          However, my expectation would be that could be expensive.
     retval = xcalloc(1, 1);
-    break;
-  }
-  case CALLABLE_NONE:
-  default:
-    abort();
   }
 
   // Remove one-shot ("once") autocmd in anticipation of its execution.
@@ -2456,60 +2460,14 @@ bool autocmd_delete_id(int64_t id)
   return success;
 }
 
-// ===========================================================================
-//  AucmdExecutable Functions
-// ===========================================================================
-
-/// Generate a string description for the command/callback of an autocmd
-char *aucmd_exec_to_string(AutoCmd *ac, AucmdExecutable acc)
+/// Gets an (allocated) string representation of an autocmd command/callback.
+char *aucmd_handler_to_string(AutoCmd *ac)
   FUNC_ATTR_PURE
 {
-  switch (acc.type) {
-  case CALLABLE_EX:
-    return xstrdup(acc.callable.cmd);
-  case CALLABLE_CB:
-    return callback_to_string(&acc.callable.cb, NULL);
-  case CALLABLE_NONE:
-    return "This is not possible";
+  if (ac->handler_cmd) {
+    return xstrdup(ac->handler_cmd);
   }
-
-  abort();
-}
-
-void aucmd_exec_free(AucmdExecutable *acc)
-{
-  switch (acc->type) {
-  case CALLABLE_EX:
-    XFREE_CLEAR(acc->callable.cmd);
-    break;
-  case CALLABLE_CB:
-    callback_free(&acc->callable.cb);
-    break;
-  case CALLABLE_NONE:
-    return;
-  }
-
-  acc->type = CALLABLE_NONE;
-}
-
-AucmdExecutable aucmd_exec_copy(AucmdExecutable src)
-{
-  AucmdExecutable dest = AUCMD_EXECUTABLE_INIT;
-
-  switch (src.type) {
-  case CALLABLE_EX:
-    dest.type = CALLABLE_EX;
-    dest.callable.cmd = xstrdup(src.callable.cmd);
-    return dest;
-  case CALLABLE_CB:
-    dest.type = CALLABLE_CB;
-    callback_copy(&dest.callable.cb, &src.callable.cb);
-    return dest;
-  case CALLABLE_NONE:
-    return dest;
-  }
-
-  abort();
+  return callback_to_string(&ac->handler_fn, NULL);
 }
 
 bool au_event_is_empty(event_T event)