refactor(ex_cmds2): re-encapsulate scriptitem_T

Followup to da9b0ab neovim#15994.
`scriptitem_T` was lifted out of ex_cmds2.c but it's not needed, so keep it
private.

Also clean up some comments from neovim#15079 and rename a few functions.
Add a test for ensuring different anon execs in the same Lua file still have
unique SIDs with tracing enabled.

src/nvim/eval.c

@@ -9284,30 +9284,29 @@ static hashtab_T *find_var_ht_dict(const char *name, const size_t name_len, cons
              && (current_sctx.sc_sid > 0 || current_sctx.sc_sid == SID_STR
                  || current_sctx.sc_sid == SID_LUA)
              && current_sctx.sc_sid <= ga_scripts.ga_len) {
-    // For anonymous scripts without a script item, create one now so script vars can be used
+    // Create SID if s: scope is accessed from Lua or anon Vimscript. #15994
     if (current_sctx.sc_sid == SID_LUA) {
-      // try to resolve lua filename & line no so it can be shown in lastset messages.
+      // Try to resolve Lua file name & line no so it can be shown in LastSet messages.
       nlua_set_sctx(&current_sctx);
       if (current_sctx.sc_sid != SID_LUA) {
-        // Great we have valid location. Now here this out we'll create a new
-        // script context with the name and lineno of this one. why ?
-        // for behavioral consistency. With this different anonymous exec from
-        // same file can't access each others script local stuff. We need to do
-        // this all other cases except this will act like that otherwise.
+        // We have a valid SID associated with a Lua file name.
+        // Create a new SID with the same fname and set it as the current.
+        // This keeps the usual behaviour of disallowing different anonymous execs
+        // in the same file from accessing each others script-local stuff.
         const LastSet last_set = (LastSet){
           .script_ctx = current_sctx,
           .channel_id = LUA_INTERNAL_CALL,
         };
+        // should_free should be true, as the script has a file name.
+        // Because script_new_sid consumes sc_name, we don't call free.
         bool should_free;
-        // should_free is ignored as script_sctx will be resolved to a fnmae
-        // & new_script_item will consume it.
-        char_u *sc_name = get_scriptname(last_set, &should_free);
-        new_script_item(sc_name, &current_sctx.sc_sid);
+        char_u *const sc_name = get_scriptname(last_set, &should_free);
+        assert(should_free);
+        current_sctx.sc_sid = script_new_sid(sc_name);
       }
     }
     if (current_sctx.sc_sid == SID_STR || current_sctx.sc_sid == SID_LUA) {
-      // Create SID if s: scope is accessed from Lua or anon Vimscript. #15994
-      new_script_item(NULL, &current_sctx.sc_sid);
+      current_sctx.sc_sid = script_new_sid(NULL);
     }
     *d = &SCRIPT_SV(current_sctx.sc_sid)->sv_dict;
   }

src/nvim/ex_cmds2.c

@@ -54,6 +54,31 @@
 #include "nvim/window.h"
 
 /// Growarray to store info about already sourced scripts.
+/// Also store the dev/ino, so that we don't have to stat() each
+/// script when going through the list.
+typedef struct scriptitem_S {
+  char_u *sn_name;
+  bool file_id_valid;
+  FileID file_id;
+  bool sn_prof_on;              ///< true when script is/was profiled
+  bool sn_pr_force;             ///< forceit: profile functions in this script
+  proftime_T sn_pr_child;       ///< time set when going into first child
+  int sn_pr_nest;               ///< nesting for sn_pr_child
+  // profiling the script as a whole
+  int sn_pr_count;              ///< nr of times sourced
+  proftime_T sn_pr_total;       ///< time spent in script + children
+  proftime_T sn_pr_self;        ///< time spent in script itself
+  proftime_T sn_pr_start;       ///< time at script start
+  proftime_T sn_pr_children;    ///< time in children after script start
+  // profiling the script per line
+  garray_T sn_prl_ga;           ///< things stored for every line
+  proftime_T sn_prl_start;      ///< start time for current line
+  proftime_T sn_prl_children;   ///< time spent in children for this line
+  proftime_T sn_prl_wait;       ///< wait start time for current line
+  linenr_T sn_prl_idx;          ///< index of line being timed; -1 if none
+  int sn_prl_execed;            ///< line being timed was executed
+} scriptitem_T;
+
 static garray_T script_items = { 0, 0, sizeof(scriptitem_T), 4, NULL };
 #define SCRIPT_ITEM(id) (((scriptitem_T *)script_items.ga_data)[(id) - 1])
 
@@ -1788,10 +1813,10 @@ static char_u *get_str_line(int c, void *cookie, int indent, bool do_concat)
 /// Create a new script item and allocate script-local vars. @see new_script_vars
 ///
 /// @param  name  File name of the script. NULL for anonymous :source.
-/// @param[out]  sid_out  SID of the new item.
+/// @param[out]  sid_out  SID of the new script.
 ///
 /// @return  pointer to the created script item.
-scriptitem_T *new_script_item(char_u *const name, scid_T *const sid_out)
+static scriptitem_T *new_script_item(char_u *const name, scid_T *const sid_out)
 {
   static scid_T last_current_SID = 0;
   const scid_T sid = ++last_current_SID;
@@ -1809,6 +1834,17 @@ scriptitem_T *new_script_item(char_u *const name, scid_T *const sid_out)
   return &SCRIPT_ITEM(sid);
 }
 
+/// Create a new ID for a script so "s:" scope vars can be used.
+/// @param  fname  allocated file name of the script. NULL for anonymous :source.
+/// @return  SID of the new script.
+scid_T script_new_sid(char_u *const fname)
+  FUNC_ATTR_WARN_UNUSED_RESULT
+{
+  scid_T sid;
+  (void)new_script_item(fname, &sid);
+  return sid;
+}
+
 static int source_using_linegetter(void *cookie, LineGetter fgetline, const char *traceback_name)
 {
   char_u *save_sourcing_name = sourcing_name;
@@ -2032,7 +2068,7 @@ int do_source(char *fname, int check_other, int is_vimrc)
   save_funccal(&funccalp_entry);
 
   const sctx_T save_current_sctx = current_sctx;
-  si = get_current_script_id(fname_exp, &current_sctx);
+  si = new_file_sctx(fname_exp, &current_sctx);
 
   if (l_do_profiling == PROF_YES) {
     bool forceit = false;
@@ -2144,13 +2180,14 @@ int do_source(char *fname, int check_other, int is_vimrc)
   return retval;
 }
 
-
-/// Check if fname was sourced before to finds its SID.
-/// If it's new, generate a new SID.
+/// Sets script context to refer to a file in preparation for sourcing and returns its item.
+/// If the file is new, a script item is created and its SID added to `file_sids`.
 ///
-/// @param[in] fname file path of script
-/// @param[out] ret_sctx sctx of this script
-scriptitem_T *get_current_script_id(char_u *fname, sctx_T *ret_sctx)
+/// @param[in]  fname  file path of script.
+/// @param[out]  ret_sctx  sctx of this script.
+/// @return  pointer to the script item.
+static scriptitem_T *new_file_sctx(char_u *fname, sctx_T *ret_sctx)
+  FUNC_ATTR_NONNULL_ARG(1)
 {
   static int last_current_SID_seq = 0;
 
@@ -2190,6 +2227,16 @@ scriptitem_T *get_current_script_id(char_u *fname, sctx_T *ret_sctx)
   return si;
 }
 
+/// Create a new context referencing a script from its file name.
+/// @param  fname  file name of script.
+/// @return  the new script context.
+sctx_T script_new_sctx(char_u *const fname)
+  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
+{
+  sctx_T sctx;
+  (void)get_script_item(fname, &sctx);
+  return sctx;
+}
 
 /// ":scriptnames"
 void ex_scriptnames(exarg_T *eap)

src/nvim/ex_cmds2.h

@@ -16,31 +16,6 @@
 #define CCGD_ALLBUF     8       // may write all buffers
 #define CCGD_EXCMD      16      // may suggest using !
 
-/// Also store the dev/ino, so that we don't have to stat() each
-/// script when going through the list.
-typedef struct scriptitem_S {
-  char_u *sn_name;
-  bool file_id_valid;
-  FileID file_id;
-  bool sn_prof_on;              ///< true when script is/was profiled
-  bool sn_pr_force;             ///< forceit: profile functions in this script
-  proftime_T sn_pr_child;       ///< time set when going into first child
-  int sn_pr_nest;               ///< nesting for sn_pr_child
-  // profiling the script as a whole
-  int sn_pr_count;              ///< nr of times sourced
-  proftime_T sn_pr_total;       ///< time spent in script + children
-  proftime_T sn_pr_self;        ///< time spent in script itself
-  proftime_T sn_pr_start;       ///< time at script start
-  proftime_T sn_pr_children;    ///< time in children after script start
-  // profiling the script per line
-  garray_T sn_prl_ga;           ///< things stored for every line
-  proftime_T sn_prl_start;      ///< start time for current line
-  proftime_T sn_prl_children;   ///< time spent in children for this line
-  proftime_T sn_prl_wait;       ///< wait start time for current line
-  linenr_T sn_prl_idx;          ///< index of line being timed; -1 if none
-  int sn_prl_execed;            ///< line being timed was executed
-} scriptitem_T;
-
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "ex_cmds2.h.generated.h"
 #endif

src/nvim/lua/executor.c

@@ -1792,6 +1792,7 @@ void nlua_execute_on_key(int c)
 // Sets the editor "script context" during Lua execution. Used by :verbose.
 // @param[out] current
 void nlua_set_sctx(sctx_T *current)
+  FUNC_ATTR_NONNULL_ALL
 {
   if (p_verbose <= 0 || current->sc_sid != SID_LUA) {
     return;
@@ -1832,7 +1833,7 @@ void nlua_set_sctx(sctx_T *current)
     break;
   }
   char *source_path = fix_fname(info->source + 1);
-  get_current_script_id((char_u *)source_path, current);
+  *current = script_new_sctx((char_u *)source_path);
   xfree(source_path);
   current->sc_lnum = info->currentline;
   current->sc_seq = -1;

test/functional/ex_cmds/source_spec.lua

@@ -140,13 +140,18 @@ describe(':source', function()
       a = [=[
         \ 1
        "\ 2]=]
+      vim.cmd [=[
+        let s:a = 3
+        let g:a = expand('<SID>')
+      ]=]
     ]])
 
     command('edit '..test_file)
     feed_command(':source')
 
     eq(12, eval('g:c'))
     eq('    \\ 1\n   "\\ 2', exec_lua('return _G.a'))
+    eq('<SNR>1_', eval('g:a'))
     os.remove(test_file)
   end)
 

test/functional/ex_cmds/verbose_spec.lua

@@ -2,6 +2,8 @@ local helpers = require('test.functional.helpers')(after_each)
 
 local clear = helpers.clear
 local eq = helpers.eq
+local neq = helpers.neq
+local eval = helpers.eval
 local exec = helpers.exec
 local exec_capture = helpers.exec_capture
 local write_file = helpers.write_file
@@ -37,13 +39,15 @@ vim.api.nvim_exec ("\
 function Close_Window() abort\
   wincmd -\
 endfunction\
+let g:sid1 = expand('<SID>')\
 ", false)
 
 local ret = vim.api.nvim_exec ("\
 function! s:return80()\
   return 80\
 endfunction\
 let &tw = s:return80()\
+let g:sid2 = expand('<SID>')\
 ", true)
 ]])
     exec(':source '..script_file)
@@ -137,8 +141,11 @@ test_group  FileType
     local result = exec_capture(':verbose set tw?')
     eq(string.format([[
   textwidth=80
-	Last set from %s line 22]],
+	Last set from %s line 23]],
        script_location), result)
+
+    -- Different anon execs in the same file should have unique SIDs as usual.
+    neq(eval('g:sid1'), eval('g:sid2'))
   end)
 end)