Skip to content

Commit

Permalink
fix(lua): sid for sourced anon vim script from lua
Browse files Browse the repository at this point in the history
Unfortunately, even after our changes that allow for <SID> to be always usable
in anonymous Vim scripts, the changes in neovim#15079 break this for Lua scripts due
to the use of SID_LUA over a unique SID in places such as nvim_exec.

Instead, use the special SID offsets to identify scripts from Lua contexts in
which nlua_set_sctx can be used. Encoding it this way avoids the need to store
such information in an allocated script item or elsewhere.

However, this approach produces ugly script IDs, so it's only used when :verbose
logging for Lua is enabled. As a result, it's now even more strongly recommended
to start nvim as `nvim -V1` rather than to set verbose=1 in the middle of a
session, otherwise tracing for previously sourced Lua scripts before verbose
was set will not work.

Remove the previous LastSet sctx dancing logic when accessing "s:" scope. It's
no longer needed as anonymous Lua scripts now get a unique SID.

Add some tests that would previously fail due to bugs with the old verbose Lua
approach.

Also, remove find_sid (unused leftover from the autocmd API PR) and change
sid_T's type to a fixed-width type (int64_t).
  • Loading branch information
seandewar committed May 13, 2022
1 parent 851dfb8 commit b572a19
Show file tree
Hide file tree
Showing 12 changed files with 186 additions and 101 deletions.
6 changes: 4 additions & 2 deletions runtime/doc/options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6756,8 +6756,10 @@ A jump table for the options with a short description can be found at |Q_op|.
global
When bigger than zero, Vim will give messages about what it is doing.
Currently, these messages are given:
>= 1 Lua assignments to options,keymaps etc.
>= 2 When a file is ":source"'ed and when the shada file is read or written..
>= 1 Lua assignments to options, keymaps, etc. (applies only to
scripts sourced after this is set).
>= 2 When a file is ":source"'ed and when the shada file is read or
written.
>= 3 UI info, terminal capabilities
>= 4 Shell commands.
>= 5 Every searched tags file and include file.
Expand Down
15 changes: 8 additions & 7 deletions runtime/doc/various.txt
Original file line number Diff line number Diff line change
Expand Up @@ -459,13 +459,14 @@ g8 Print the hex values of the bytes used in the
'verbosefile' option.

*:verbose-cmd*
When 'verbose' is non-zero, listing the value of a Vim option or a key map or
an abbreviation or a user-defined function or a command or a highlight group
or an autocommand will also display where it was last defined. If they were
defined in Lua they will only be located if 'verbose' is set. So Start
nvim with -V1 arg to see them. If it was defined manually then there
will be no "Last set" message. When it was defined while executing a function,
user command or autocommand, the script in which it was defined is reported.
When 'verbose' is non-zero, listing the value of a Vim option, key map,
abbreviation, user-defined function or command, highlight group or an
autocommand will also display where it was last defined. If they were defined
from Lua they will be located if 'verbose' was set before the script was
sourced, so it is recommended to start Nvim with the "-V1" argument to see
them. If it was defined manually then there will be no "Last set" message.
When it was defined while executing a function, user command or autocommand,
the script in which it was defined is reported.

*K*
[count]K Runs the program given by 'keywordprg' to lookup the
Expand Down
16 changes: 1 addition & 15 deletions src/nvim/api/private/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -1626,19 +1626,6 @@ void create_user_command(String name, Object command, Dict(user_command) *opts,
NLUA_CLEAR_REF(compl_luaref);
}

int find_sid(uint64_t channel_id)
{
switch (channel_id) {
case VIML_INTERNAL_CALL:
// TODO(autocmd): Figure out what this should be
// return SID_API_CLIENT;
case LUA_INTERNAL_CALL:
return SID_LUA;
default:
return SID_API_CLIENT;
}
}

/// Sets sctx for API calls.
///
/// @param channel_id api clients id. Used to determine if it's a internal
Expand All @@ -1649,8 +1636,7 @@ sctx_T api_set_sctx(uint64_t channel_id)
{
sctx_T old_current_sctx = current_sctx;
if (channel_id != VIML_INTERNAL_CALL) {
current_sctx.sc_sid =
channel_id == LUA_INTERNAL_CALL ? SID_LUA : SID_API_CLIENT;
current_sctx.sc_sid = channel_id == LUA_INTERNAL_CALL ? SID_LUA : SID_API_CLIENT;
current_sctx.sc_lnum = 0;
}
return old_current_sctx;
Expand Down
28 changes: 2 additions & 26 deletions src/nvim/eval.c
Original file line number Diff line number Diff line change
Expand Up @@ -6170,7 +6170,7 @@ void common_function(typval_T *argvars, typval_T *rettv, bool is_funcref, FunPtr
int arg_idx = 0;
list_T *list = NULL;
if (STRNCMP(s, "s:", 2) == 0 || STRNCMP(s, "<SID>", 5) == 0) {
char sid_buf[25];
char sid_buf[SIDBUFLEN];
int off = *s == 's' ? 2 : 5;

// Expand s: and <SID> into <SNR>nr_, so that the function can
Expand Down Expand Up @@ -8829,31 +8829,7 @@ static hashtab_T *find_var_ht_dict(const char *name, const size_t name_len, cons
*d = &funccal->l_avars;
} else if (*name == 'l' && funccal != NULL) { // local variable
*d = &funccal->l_vars;
} else if (*name == 's' // script variable
&& (current_sctx.sc_sid > 0 || current_sctx.sc_sid == SID_LUA)) {
// Create SID if s: scope is accessed from Lua or anon Vimscript. #15994
if (current_sctx.sc_sid == SID_LUA) {
// 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) {
// 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;
char *const sc_name = (char *)get_scriptname(last_set, &should_free);
assert(should_free);
current_sctx.sc_sid = script_new_sid(sc_name);
} else {
current_sctx.sc_sid = script_new_sid(NULL);
}
}
} else if (*name == 's' && current_sctx.sc_sid > 0) { // script variable
// "s:" hash map is lazily allocated; ensure it's allocated now.
scriptvar_T *sv = script_sv(current_sctx.sc_sid);
if (sv == NULL) {
Expand Down
6 changes: 4 additions & 2 deletions src/nvim/eval/typval.h
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,11 @@ struct blobvar_S {
};

/// Type used for script ID
typedef int scid_T;
typedef int64_t scid_T;
/// Format argument for scid_T
#define PRIdSCID "d"
#define PRIdSCID PRId64
/// Buffer size required to hold "<SNR>", a (not special) script ID, underscore and NUL.
#define SIDBUFLEN (26)

// SCript ConteXt (SCTX): identifies a script line.
// When sourcing a script "sc_lnum" is zero, "sourcing_lnum" is the current
Expand Down
4 changes: 2 additions & 2 deletions src/nvim/eval/userfunc.c
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ int get_func_tv(const char_u *name, int len, typval_T *rettv, char_u **arg, func
return ret;
}

#define FLEN_FIXED 40
#define FLEN_FIXED (SIDBUFLEN + 28)

/// Check whether function name starts with <SID> or s:
///
Expand Down Expand Up @@ -1845,7 +1845,7 @@ char_u *trans_function_name(char_u **pp, bool skip, int flags, funcdict_T *fdp,
}

size_t sid_buf_len = 0;
char sid_buf[20];
char sid_buf[SIDBUFLEN];

// Copy the function name to allocated memory.
// Accept <SID>name() inside a script, translate into <SNR>123_name().
Expand Down
77 changes: 52 additions & 25 deletions src/nvim/ex_cmds2.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ typedef struct scriptitem_S {
int sn_prl_execed; ///< line being timed was executed
} scriptitem_T;

static scid_T last_current_SID = 0;
static int last_current_SID_seq = 0;
static PMap(scid_T) script_items = MAP_INIT;

Expand Down Expand Up @@ -1845,16 +1844,26 @@ static scriptitem_T *new_script_item(const scid_T id, char *const name)
return si;
}

/// 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 *const fname)
/// @param trace_type When `nlua_is_verbose` is true, encodes tracing info in the
/// returned SID. Used by `nlua_set_sctx` for :verbose logging.
/// @return a new script ID.
scid_T script_new_sid(const SidTraceType trace_type)
FUNC_ATTR_WARN_UNUSED_RESULT
{
const scid_T sid = ++last_current_SID;
// Anonymous :sources don't require an allocated script item.
if (fname != NULL) {
(void)new_script_item(sid, fname);
static scid_T last_current_SID = 0;

scid_T sid = ++last_current_SID;
if (trace_type != kSidTraceNone && nlua_is_verbose()) {
switch (trace_type) {
case kSidTraceLua:
sid += SID_TRACE_LUA;
break;
case kSidTraceFromLua:
sid += SID_TRACE_FROM_LUA;
break;
default:
abort();
}
}
return sid;
}
Expand Down Expand Up @@ -1915,13 +1924,14 @@ static void cmd_source_buffer(const exarg_T *const eap)
.buf = ga.ga_data,
.offset = 0,
};

if (curbuf->b_fname && path_with_extension((const char *)curbuf->b_fname, "lua")) {
nlua_source_using_linegetter(get_str_line, (void *)&cookie, ":source (no file)");
} else {
scid_T sid = map_get(handle_T, scid_T)(&buf_sids, curbuf->handle);
if (sid == 0) {
// First time sourcing this buffer, use a new SID for it.
sid = ++last_current_SID;
sid = script_new_sid(kSidTraceNone);
map_put(handle_T, scid_T)(&buf_sids, curbuf->handle, sid);
}
source_using_linegetter((void *)&cookie, get_str_line, ":source (no file)", sid);
Expand All @@ -1938,8 +1948,10 @@ int do_source_str(const char *cmd, const char *traceback_name)
.buf = (char *)cmd,
.offset = 0,
};
const scid_T sid = current_sctx.sc_sid == SID_LUA ? SID_LUA : ++last_current_SID;
return source_using_linegetter((void *)&cookie, get_str_line, traceback_name, sid);
const scid_T sid = script_new_sid(
current_sctx.sc_sid == SID_LUA || current_sctx.sc_sid >= SID_TRACE_LUA
? kSidTraceFromLua : kSidTraceNone);
return source_using_linegetter(&cookie, get_str_line, traceback_name, sid);
}

/// When fname is a 'lua' file nlua_exec_file() is invoked to source it.
Expand Down Expand Up @@ -2087,7 +2099,8 @@ int do_source(char *fname, int check_other, int is_vimrc)
save_funccal(&funccalp_entry);

const sctx_T save_current_sctx = current_sctx;
si = new_file_sctx((char_u *)fname_exp, &current_sctx);
const bool is_lua = path_with_extension(fname, "lua");
si = new_file_sctx((char_u *)fname_exp, &current_sctx, is_lua);

if (l_do_profiling == PROF_YES) {
bool forceit = false;
Expand Down Expand Up @@ -2120,10 +2133,9 @@ int do_source(char *fname, int check_other, int is_vimrc)
firstline = (uint8_t *)p;
}

if (path_with_extension((const char *)fname_exp, "lua")) {
if (is_lua) {
const sctx_T current_sctx_backup = current_sctx;
const linenr_T sourcing_lnum_backup = sourcing_lnum;
current_sctx.sc_sid = SID_LUA;
current_sctx.sc_lnum = 0;
sourcing_lnum = 0;
// Source the file as lua
Expand Down Expand Up @@ -2202,8 +2214,9 @@ int do_source(char *fname, int check_other, int is_vimrc)
///
/// @param[in] fname file path of script.
/// @param[out] ret_sctx sctx of this script.
/// @param is_lua true if a Lua script.
/// @return pointer to the script item.
static scriptitem_T *new_file_sctx(char_u *fname, sctx_T *ret_sctx)
static scriptitem_T *new_file_sctx(char_u *const fname, sctx_T *const ret_sctx, const bool is_lua)
FUNC_ATTR_NONNULL_ARG(1)
{
sctx_T script_sctx = { .sc_seq = ++last_current_SID_seq,
Expand All @@ -2226,7 +2239,7 @@ static scriptitem_T *new_file_sctx(char_u *fname, sctx_T *ret_sctx)
}
}
if (fi < 0) {
script_sctx.sc_sid = ++last_current_SID;
script_sctx.sc_sid = script_new_sid(is_lua ? kSidTraceLua : kSidTraceNone);
si = new_script_item(script_sctx.sc_sid, (char *)vim_strsave(fname));
GA_APPEND(scid_T, &file_sids, script_sctx.sc_sid);
if (file_id_ok) {
Expand All @@ -2241,15 +2254,28 @@ static scriptitem_T *new_file_sctx(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 *const fname)
FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
/// @param sid ID of the script.
/// @return Name of the script. NULL for anonymous scripts and special SIDs.
char_u *script_name(const scid_T sid)
FUNC_ATTR_WARN_UNUSED_RESULT
{
const scriptitem_T *const si = script_item(sid);
return si != NULL ? si->sn_name : NULL;
}

/// Set the name of a script.
/// @note Allocates a script item for scripts without one.
/// @param sid ID of the script.
/// @param name Allocated new name of the script, or NULL to make it anonymous.
void script_set_name(const scid_T sid, char *const name)
{
sctx_T sctx;
(void)new_file_sctx((char_u *)fname, &sctx);
return sctx;
scriptitem_T *const si = script_item(sid);
if (si != NULL) {
xfree(si->sn_name);
si->sn_name = (char_u *)name;
} else {
new_script_item(sid, name);
}
}

/// ":scriptnames"
Expand Down Expand Up @@ -2316,6 +2342,7 @@ char_u *get_scriptname(LastSet last_set, bool *should_free)
case SID_STR:
return (char_u *)_("anonymous :source");
default: {
assert(last_set.script_ctx.sc_sid > 0);
const scriptitem_T *const si = script_item(last_set.script_ctx.sc_sid);
if (si == NULL || si->sn_name == NULL) {
snprintf((char *)IObuff, IOSIZE, _("anonymous :source (script id %" PRIdSCID ")"),
Expand Down
6 changes: 6 additions & 0 deletions src/nvim/ex_cmds2.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
#define CCGD_ALLBUF 8 // may write all buffers
#define CCGD_EXCMD 16 // may suggest using !

typedef enum {
kSidTraceNone,
kSidTraceLua,
kSidTraceFromLua,
} SidTraceType;

#ifdef INCLUDE_GENERATED_DECLARATIONS
# include "ex_cmds2.h.generated.h"
#endif
Expand Down
5 changes: 5 additions & 0 deletions src/nvim/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,11 @@ EXTERN int garbage_collect_at_exit INIT(= false);
#define SID_API_CLIENT (-9) // for API clients
#define SID_STR (-10) // for sourcing a string with no script item

// Offset values for new SIDs used for tracing purposes.
// Only used when `nlua_is_verbose` returns true.
#define SID_TRACE_FROM_LUA ((scid_T)1 << 61) ///< SID created by a Lua script.
#define SID_TRACE_LUA ((scid_T)1 << 62) ///< SID refers to a Lua script.

// Script CTX being sourced or was sourced to define the current function.
EXTERN sctx_T current_sctx INIT(= { 0 COMMA 0 COMMA 0 });
// ID of the current channel making a client API call
Expand Down
Loading

0 comments on commit b572a19

Please sign in to comment.