Skip to content

Commit

Permalink
fix(lua): allow SID to be used in anonymous sources
Browse files Browse the repository at this point in the history
TODO: the nlua_set_sctx stuff is more bugged than i thought...

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 mask SID_MASK_LUA to identify scripts from Lua
contexts. Encoding it this way avoids the need to store such information in
an allocated script item or elsewhere.
(Though it does produce some ugly script IDs...)

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

Also, remove find_sid (unused leftover stuff from the autocmd API PR) and change
sid_T's type to int64_t.
  • Loading branch information
seandewar committed Mar 28, 2022
1 parent edb5314 commit 43e2c1d
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 61 deletions.
17 changes: 2 additions & 15 deletions src/nvim/api/private/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "nvim/decoration.h"
#include "nvim/eval.h"
#include "nvim/eval/typval.h"
#include "nvim/ex_cmds2.h"
#include "nvim/extmark.h"
#include "nvim/fileio.h"
#include "nvim/getchar.h"
Expand Down Expand Up @@ -1613,19 +1614,6 @@ void add_user_command(String name, Object command, Dict(user_command) *opts, int
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 @@ -1636,8 +1624,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 @@ -6638,7 +6638,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 @@ -9296,31 +9296,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_u *const sc_name = 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 @@ -1844,7 +1844,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
23 changes: 16 additions & 7 deletions src/nvim/ex_cmds2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1847,18 +1847,26 @@ static scriptitem_T *new_script_item(const scid_T id, char_u *const name)

/// 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.
/// @param is_lua true if the script is created within a Lua context.
/// @return SID of the new script.
scid_T script_new_sid(char_u *const fname)
scid_T script_new_sid(char_u *const fname, const bool is_lua)
FUNC_ATTR_WARN_UNUSED_RESULT
{
const scid_T sid = ++last_current_SID;
const scid_T sid = ++last_current_SID | (is_lua ? SID_MASK_LUA : 0);
// Anonymous :sources don't require an allocated script item.
if (fname != NULL) {
(void)new_script_item(sid, fname);
}
return sid;
}

/// @return true if a SID refers to a Lua script.
bool sid_is_lua(const scid_T sid)
FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_CONST
{
return sid == SID_LUA || (sid > 0 && (sid & SID_MASK_LUA));
}

static int source_using_linegetter(void *cookie, LineGetter fgetline, const char *traceback_name,
const scid_T sid)
{
Expand Down Expand Up @@ -1940,8 +1948,8 @@ int do_source_str(const char *cmd, const char *traceback_name)
.offset = 0,
.sourcing_lnum = 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 = ++last_current_SID | (current_sctx.sc_sid == SID_LUA ? SID_MASK_LUA : 0);
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 @@ -2122,10 +2130,9 @@ int do_source(char *fname, int check_other, int is_vimrc)
firstline = p;
}

if (path_with_extension((const char *)fname_exp, "lua")) {
if (current_sctx.sc_sid & SID_MASK_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 @@ -2228,7 +2235,8 @@ static scriptitem_T *get_script_item(char_u *fname, sctx_T *ret_sctx)
}
}
if (i < 0) {
script_sctx.sc_sid = ++last_current_SID;
const bool is_lua = path_with_extension((const char *)fname, "lua");
script_sctx.sc_sid = ++last_current_SID | (is_lua ? SID_MASK_LUA : 0);
si = new_script_item(script_sctx.sc_sid, vim_strsave(fname));
GA_APPEND(scid_T, &file_sids, script_sctx.sc_sid);
if (file_id_ok) {
Expand Down Expand Up @@ -2318,6 +2326,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
3 changes: 3 additions & 0 deletions src/nvim/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,9 @@ 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

/// Mask for an anonymous script with a unique SID created within a Lua context.
#define SID_MASK_LUA ((scid_T)1 << 62)

// 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
4 changes: 2 additions & 2 deletions src/nvim/lua/executor.c
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ int nlua_source_using_linegetter(LineGetter fgetline, void *cookie, char *name)
{
const linenr_T save_sourcing_lnum = sourcing_lnum;
const sctx_T save_current_sctx = current_sctx;
current_sctx.sc_sid = SID_STR;
current_sctx.sc_sid = script_new_sid(NULL, true);
current_sctx.sc_seq = 0;
current_sctx.sc_lnum = 0;
sourcing_lnum = 0;
Expand Down Expand Up @@ -1794,7 +1794,7 @@ void nlua_execute_on_key(int c)
void nlua_set_sctx(sctx_T *current)
FUNC_ATTR_NONNULL_ALL
{
if (p_verbose <= 0 || current->sc_sid != SID_LUA) {
if (p_verbose <= 0 || !sid_is_lua(current->sc_sid)) {
return;
}
lua_State *const lstate = global_lstate;
Expand Down
5 changes: 5 additions & 0 deletions test/functional/api/vim_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,11 @@ describe('API', function()
eq(NIL, meths.exec_lua('function xx(a,b)\nreturn a..b\nend',{}))
eq("xy", meths.exec_lua('return xx(...)', {'x','y'}))

eq("<SNR>4611686018427387905_", meths.exec_lua([[
vim.api.nvim_exec("let g:a = expand('<SID>')", false)
return vim.g.a
]], {}))

-- Deprecated name: nvim_execute_lua.
eq("xy", meths.execute_lua('return xx(...)', {'x','y'}))
end)
Expand Down
2 changes: 1 addition & 1 deletion test/functional/ex_cmds/source_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe(':source', function()

eq(12, eval('g:c'))
eq(' \\ 1\n "\\ 2', exec_lua('return _G.a'))
eq('<SNR>1_', eval('g:a'))
eq('<SNR>4611686018427387906_', eval('g:a'))
os.remove(test_file)
end)

Expand Down
16 changes: 10 additions & 6 deletions test/functional/ex_cmds/verbose_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ vim.keymap.set('n', '<leader>key2', ':echo "test"<cr>')
vim.api.nvim_exec("augroup test_group\
autocmd!\
autocmd FileType c setl cindent\
autocmd User Foo let s:test += 1 | echom s:test\
augroup END\
let s:test = 1336\
", false)
vim.api.nvim_command("command Bdelete :bd")
Expand Down Expand Up @@ -107,14 +109,16 @@ test_group FileType
c setl cindent
Last set from %s line 7]],
script_location), result)

eq("1337", exec_capture(":doautocmd User Foo"))
end)

it('"Last set" for command defined by nvim_command', function()
local result = exec_capture(':verbose command Bdelete')
eq(string.format([[
Name Args Address Complete Definition
Bdelete 0 :bd
Last set from %s line 13]],
Last set from %s line 15]],
script_location), result)
end)

Expand All @@ -123,15 +127,15 @@ test_group FileType
eq(string.format([[
Name Args Address Complete Definition
TestCommand 0 :echo 'Hello'
Last set from %s line 14]],
Last set from %s line 16]],
script_location), result)
end)

it('"Last set for function', function()
it('"Last set" for function', function()
local result = exec_capture(':verbose function Close_Window')
eq(string.format([[
function Close_Window() abort
Last set from %s line 16
Last set from %s line 18
1 wincmd -
endfunction]],
script_location), result)
Expand All @@ -143,14 +147,14 @@ test_group FileType
-- "Last set from anonymous :source (script id <blah>) line 5 in <script_location> line 23"
eq(string.format([[
textwidth=80
Last set from %s line 28]],
Last set from %s line 30]],
script_location), result)

-- Different anon execs in the same file should have unique SIDs as usual.
neq(eval('g:sid1'), eval('g:sid2'))

-- Also shouldn't add duplicate names to :scriptnames output.
eq(string.format(" 2: %s", script_location), exec_capture(':scriptnames'))
eq(string.format("4611686018427387906: %s", script_location), exec_capture(':scriptnames'))
end)
end)

Expand Down

0 comments on commit 43e2c1d

Please sign in to comment.