IntenseFinance/neovim
3
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2025-02-25 18:55:25 -06:00
Code Issues Packages Projects Releases Wiki Activity

refactor(api): always use TRY_WRAP #31600

Browse Source 
Problem:  Two separate try/end wrappers, that only marginally differ by
          restoring a few variables. Wrappers that don't restore
          previous state are dangerous to use in "api-fast" functions.
Solution: Remove wrappers that don't restore the previous state.
          Always use TRY_WRAP.
This commit is contained in:
luukvbaal 2024-12-17 13:12:22 +01:00 committed by GitHub
parent b03e790cdd
commit 6bf2a6fc5b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 377 additions and 427 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/nvim/api/buffer.c
View File

@ -360,8 +360,7 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
memchrsub(lines[i], NUL, NL, l.size);
}
try_start();
TRY_WRAP(err, {
if (!MODIFIABLE(buf)) {
api_set_error(err, kErrorTypeException, "Buffer is not 'modifiable'");
goto end;
@ -444,9 +443,8 @@ void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integ
fix_cursor(win, (linenr_T)start, (linenr_T)end, (linenr_T)extra);
}
}
end:
try_end(err);
end:;
});
}
/// Sets (replaces) a range in the buffer
@ -593,8 +591,7 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
new_byte += (bcount_t)(last_item.size) + 1;
}
try_start();
TRY_WRAP(err, {
if (!MODIFIABLE(buf)) {
api_set_error(err, kErrorTypeException, "Buffer is not 'modifiable'");
goto end;
@ -685,9 +682,8 @@ void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, In
}
}
}
end:
try_end(err);
end:;
});
}
/// Gets a range from the buffer.
@ -965,8 +961,8 @@ void nvim_buf_set_name(Buffer buffer, String name, Error *err)
return;
}
try_start();
int ren_ret = OK;
TRY_WRAP(err, {
const bool is_curbuf = buf == curbuf;
const int save_acd = p_acd;
if (!is_curbuf) {
@ -977,14 +973,15 @@ void nvim_buf_set_name(Buffer buffer, String name, Error *err)
// Using aucmd_*: autocommands will be executed by rename_buffer
aco_save_T aco;
aucmd_prepbuf(&aco, buf);
int ren_ret = rename_buffer(name.data);
ren_ret = rename_buffer(name.data);
aucmd_restbuf(&aco);
if (!is_curbuf) {
p_acd = save_acd;
}
});
if (try_end(err)) {
if (ERROR_SET(err)) {
return;
}
@ -1204,15 +1201,18 @@ Object nvim_buf_call(Buffer buffer, LuaRef fun, Error *err)
if (!buf) {
return NIL;
}
try_start();
Object res = OBJECT_INIT;
TRY_WRAP(err, {
aco_save_T aco;
aucmd_prepbuf(&aco, buf);
Array args = ARRAY_DICT_INIT;
Object res = nlua_call_ref(fun, NULL, args, kRetLuaref, NULL, err);
res = nlua_call_ref(fun, NULL, args, kRetLuaref, NULL, err);
aucmd_restbuf(&aco);
try_end(err);
});
return res;
}

79
src/nvim/api/private/helpers.c
View File

@ -42,8 +42,10 @@
/// Start block that may cause Vimscript exceptions while evaluating another code
///
/// Used when caller is supposed to be operating when other Vimscript code is being
/// processed and that “other Vimscript code” must not be affected.
/// Used just in case caller is supposed to be operating when other Vimscript code
/// is being processed and that “other Vimscript code” must not be affected.
///
/// @warning Avoid calling directly; use TRY_WRAP instead.
///
/// @param[out] tstate Location where try state should be saved.
void try_enter(TryState *const tstate)
@ -55,75 +57,33 @@ void try_enter(TryState *const tstate)
.current_exception = current_exception,
.msg_list = (const msglist_T *const *)msg_list,
.private_msg_list = NULL,
.trylevel = trylevel,
.got_int = got_int,
.did_throw = did_throw,
.need_rethrow = need_rethrow,
.did_emsg = did_emsg,
};
// `msg_list` controls the collection of abort-causing non-exception errors,
// which would otherwise be ignored. This pattern is from do_cmdline().
msg_list = &tstate->private_msg_list;
current_exception = NULL;
trylevel = 1;
got_int = false;
did_throw = false;
need_rethrow = false;
did_emsg = false;
}
/// End try block, set the error message if any and restore previous state
///
/// @warning Return is consistent with most functions (false on error), not with
/// try_end (true on error).
///
/// @param[in] tstate Previous state to restore.
/// @param[out] err Location where error should be saved.
///
/// @return false if error occurred, true otherwise.
bool try_leave(const TryState *const tstate, Error *const err)
FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
{
const bool ret = !try_end(err);
assert(trylevel == 0);
assert(!need_rethrow);
assert(!got_int);
assert(!did_throw);
assert(!did_emsg);
assert(msg_list == &tstate->private_msg_list);
assert(*msg_list == NULL);
assert(current_exception == NULL);
msg_list = (msglist_T **)tstate->msg_list;
current_exception = tstate->current_exception;
trylevel = tstate->trylevel;
got_int = tstate->got_int;
did_throw = tstate->did_throw;
need_rethrow = tstate->need_rethrow;
did_emsg = tstate->did_emsg;
return ret;
}
/// Starts a block that may cause Vimscript exceptions; must be mirrored by `try_end()` call.
///
/// Note: use `TRY_WRAP` instead (except in `FUNC_API_FAST` functions such as nvim_get_runtime_file).
///
/// To be used as a replacement of `:try … catch … endtry` in C code, in cases
/// when error flag could not already be set. If there may be pending error
/// state at the time try_start() is executed which needs to be preserved,
/// try_enter()/try_leave() pair should be used instead.
void try_start(void)
{
trylevel++;
}
/// Ends a `try_start` block; sets error message if any and returns true if an error occurred.
/// Ends a `try_enter` block; sets error message if any.
///
/// Note: use `TRY_WRAP` instead (except in `FUNC_API_FAST` functions such as nvim_get_runtime_file).
/// @warning Avoid calling directly; use TRY_WRAP instead.
///
/// @param err Pointer to the stack-allocated error object
/// @return true if an error occurred
bool try_end(Error *err)
/// @param[out] err Pointer to the stack-allocated error object
void try_leave(const TryState *const tstate, Error *const err)
FUNC_ATTR_NONNULL_ALL
{
// Note: all globals manipulated here should be saved/restored in
// try_enter/try_leave.
assert(trylevel > 0);
trylevel--;
// Set by emsg(), affects aborting(). See also enter_cleanup().
@ -166,7 +126,20 @@ bool try_end(Error *err)
discard_current_exception();
}
return ERROR_SET(err);
assert(msg_list == &tstate->private_msg_list);
assert(*msg_list == NULL);
assert(current_exception == NULL);
assert(!got_int);
assert(!did_throw);
assert(!need_rethrow);
assert(!did_emsg);
// Restore the exception context.
msg_list = (msglist_T **)tstate->msg_list;
current_exception = tstate->current_exception;
got_int = tstate->got_int;
did_throw = tstate->did_throw;
need_rethrow = tstate->need_rethrow;
did_emsg = tstate->did_emsg;
}
/// Recursively expands a vimscript value in a dict

14
src/nvim/api/private/helpers.h
View File

@ -147,27 +147,19 @@ typedef struct {
except_T *current_exception;
msglist_T *private_msg_list;
const msglist_T *const *msg_list;
int trylevel;
int got_int;
bool did_throw;
int need_rethrow;
int did_emsg;
} TryState;
// `msg_list` controls the collection of abort-causing non-exception errors,
// which would otherwise be ignored. This pattern is from do_cmdline().
//
// TODO(bfredl): prepare error-handling at "top level" (nv_event).
#define TRY_WRAP(err, code) \
do { \
msglist_T **saved_msg_list = msg_list; \
msglist_T *private_msg_list; \
msg_list = &private_msg_list; \
private_msg_list = NULL; \
try_start(); \
TryState tstate; \
try_enter(&tstate); \
code; \
try_end(err); \
msg_list = saved_msg_list; /* Restore the exception context. */ \
try_leave(&tstate, err); \
} while (0)
// Execute code with cursor position saved and restored and textlock active.

6
src/nvim/api/tabpage.c
View File

@ -146,11 +146,9 @@ void nvim_tabpage_set_win(Tabpage tabpage, Window win, Error *err)
}
if (tp == curtab) {
try_start();
TRY_WRAP(err, {
win_goto(wp);
if (!try_end(err) && curwin != wp) {
api_set_error(err, kErrorTypeException, "Failed to switch to window %d", win);
}
});
} else if (tp->tp_curwin != wp) {
tp->tp_prevwin = tp->tp_curwin;
tp->tp_curwin = wp;

20
src/nvim/api/vim.c
View File

@ -594,12 +594,10 @@ ArrayOf(String) nvim_get_runtime_file(String name, Boolean all, Arena *arena, Er
kvi_init(cookie.rv);
int flags = DIP_DIRFILE | (all ? DIP_ALL : 0);
TryState tstate;
// XXX: intentionally not using `TRY_WRAP`, to avoid `did_emsg=false` in `try_end`.
try_enter(&tstate);
TRY_WRAP(err, {
do_in_runtimepath((name.size ? name.data : ""), flags, find_runtime_cb, &cookie);
vim_ignored = try_leave(&tstate, err);
});
return arena_take_arraybuilder(arena, &cookie.rv);
}
@ -952,7 +950,9 @@ void nvim_set_current_win(Window window, Error *err)
Buffer nvim_create_buf(Boolean listed, Boolean scratch, Error *err)
FUNC_API_SINCE(6)
{
try_start();
Buffer ret = 0;
TRY_WRAP(err, {
// Block autocommands for now so they don't mess with the buffer before we
// finish configuring it.
block_autocmds();
@ -1006,14 +1006,14 @@ Buffer nvim_create_buf(Boolean listed, Boolean scratch, Error *err)
goto fail;
}
try_end(err);
return buf->b_fnum;
ret = buf->b_fnum;
fail:;
});
fail:
if (!try_end(err)) {
if (ret == 0 && !ERROR_SET(err)) {
api_set_error(err, kErrorTypeException, "Failed to create buffer");
}
return 0;
return ret;
}
/// Open a terminal instance in a buffer

4
src/nvim/api/vimscript.c
View File

@ -78,7 +78,7 @@ String exec_impl(uint64_t channel_id, String src, Dict(exec_opts) *opts, Error *
capture_ga = &capture_local;
}
try_start();
TRY_WRAP(err, {
if (opts->output) {
msg_silent++;
msg_col = 0; // prevent leading spaces
@ -95,7 +95,7 @@ String exec_impl(uint64_t channel_id, String src, Dict(exec_opts) *opts, Error *
}
current_sctx = save_current_sctx;
try_end(err);
});
if (ERROR_SET(err)) {
goto theend;

12
src/nvim/api/window.c
View File

@ -368,9 +368,7 @@ void nvim_win_hide(Window window, Error *err)
}
tabpage_T *tabpage = win_find_tabpage(win);
TryState tstate;
try_enter(&tstate);
TRY_WRAP(err, {
// Never close the autocommand window.
if (is_aucmd_win(win)) {
emsg(_(e_autocmd_close));
@ -379,8 +377,7 @@ void nvim_win_hide(Window window, Error *err)
} else {
win_close_othertab(win, false, tabpage);
}
vim_ignored = try_leave(&tstate, err);
});
}
/// Closes the window (like |:close| with a |window-ID|).
@ -400,10 +397,9 @@ void nvim_win_close(Window window, Boolean force, Error *err)
}
tabpage_T *tabpage = win_find_tabpage(win);
TryState tstate;
try_enter(&tstate);
TRY_WRAP(err, {
ex_win_close(force, win, tabpage == curtab ? NULL : tabpage);
vim_ignored = try_leave(&tstate, err);
});
}
/// Calls a function with window as temporary current window.

59
src/nvim/ex_getln.c
View File

@ -787,9 +787,7 @@ static uint8_t *command_line_enter(int firstc, int count, int indent, bool clear
setmouse();
setcursor();
TryState tstate;
Error err = ERROR_INIT;
bool tl_ret = true;
char firstcbuf[2];
firstcbuf[0] = (char)(firstc > 0 ? firstc : '-');
firstcbuf[1] = 0;
@ -802,20 +800,19 @@ static uint8_t *command_line_enter(int firstc, int count, int indent, bool clear
tv_dict_add_str(dict, S_LEN("cmdtype"), firstcbuf);
tv_dict_add_nr(dict, S_LEN("cmdlevel"), ccline.level);
tv_dict_set_keys_readonly(dict);
try_enter(&tstate);
TRY_WRAP(&err, {
apply_autocmds(EVENT_CMDLINEENTER, firstcbuf, firstcbuf, false, curbuf);
restore_v_event(dict, &save_v_event);
});
tl_ret = try_leave(&tstate, &err);
if (!tl_ret && ERROR_SET(&err)) {
if (ERROR_SET(&err)) {
msg_putchar('\n');
msg_scroll = true;
msg_puts_hl(err.msg, HLF_E, true);
api_clear_error(&err);
redrawcmd();
}
tl_ret = true;
err = ERROR_INIT;
}
may_trigger_modechanged();
@ -873,10 +870,10 @@ static uint8_t *command_line_enter(int firstc, int count, int indent, bool clear
// not readonly:
tv_dict_add_bool(dict, S_LEN("abort"),
s->gotesc ? kBoolVarTrue : kBoolVarFalse);
try_enter(&tstate);
TRY_WRAP(&err, {
apply_autocmds(EVENT_CMDLINELEAVE, firstcbuf, firstcbuf, false, curbuf);
// error printed below, to avoid redraw issues
tl_ret = try_leave(&tstate, &err);
});
if (tv_dict_get_number(dict, "abort") != 0) {
s->gotesc = true;
}
@ -929,7 +926,7 @@ static uint8_t *command_line_enter(int firstc, int count, int indent, bool clear
msg_scroll = s->save_msg_scroll;
redir_off = false;
if (!tl_ret && ERROR_SET(&err)) {
if (ERROR_SET(&err)) {
msg_putchar('\n');
emsg(err.msg);
did_emsg = false;
@ -937,7 +934,7 @@ static uint8_t *command_line_enter(int firstc, int count, int indent, bool clear
}
// When the command line was typed, no need for a wait-return prompt.
if (s->some_key_typed && tl_ret) {
if (s->some_key_typed && !ERROR_SET(&err)) {
need_wait_return = false;
}
@ -2315,11 +2312,13 @@ static win_T *cmdpreview_open_win(buf_T *cmdpreview_buf)
win_T *preview_win = curwin;
Error err = ERROR_INIT;
int result = OK;
// Switch to preview buffer
try_start();
int result = do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, cmdpreview_buf->handle, 0);
if (try_end(&err) || result == FAIL) {
TRY_WRAP(&err, {
result = do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, cmdpreview_buf->handle, 0);
});
if (ERROR_SET(&err) || result == FAIL) {
api_clear_error(&err);
return NULL;
}
@ -2600,9 +2599,10 @@ static bool cmdpreview_may_show(CommandLineState *s)
// open the preview window. The preview callback also handles doing the changes and highlights for
// the preview.
Error err = ERROR_INIT;
try_start();
TRY_WRAP(&err, {
cmdpreview_type = execute_cmd(&ea, &cmdinfo, true);
if (try_end(&err)) {
});
if (ERROR_SET(&err)) {
api_clear_error(&err);
cmdpreview_type = 0;
}
@ -2643,7 +2643,6 @@ end:
static void do_autocmd_cmdlinechanged(int firstc)
{
if (has_event(EVENT_CMDLINECHANGED)) {
TryState tstate;
Error err = ERROR_INIT;
save_v_event_T save_v_event;
dict_T *dict = get_v_event(&save_v_event);
@ -2656,13 +2655,11 @@ static void do_autocmd_cmdlinechanged(int firstc)
tv_dict_add_str(dict, S_LEN("cmdtype"), firstcbuf);
tv_dict_add_nr(dict, S_LEN("cmdlevel"), ccline.level);
tv_dict_set_keys_readonly(dict);
try_enter(&tstate);
TRY_WRAP(&err, {
apply_autocmds(EVENT_CMDLINECHANGED, firstcbuf, firstcbuf, false, curbuf);
restore_v_event(dict, &save_v_event);
bool tl_ret = try_leave(&tstate, &err);
if (!tl_ret && ERROR_SET(&err)) {
});
if (ERROR_SET(&err)) {
msg_putchar('\n');
msg_scroll = true;
msg_puts_hl(err.msg, HLF_E, true);
@ -3179,11 +3176,9 @@ static bool color_cmdline(CmdlineInfo *colored_ccline)
static int prev_prompt_errors = 0;
Callback color_cb = CALLBACK_NONE;
bool can_free_cb = false;
TryState tstate;
Error err = ERROR_INIT;
const char *err_errmsg = e_intern2;
bool dgc_ret = true;
bool tl_ret = true;
if (colored_ccline->prompt_id != prev_prompt_id) {
prev_prompt_errors = 0;
@ -3196,16 +3191,16 @@ static bool color_cmdline(CmdlineInfo *colored_ccline)
assert(colored_ccline->input_fn);
color_cb = colored_ccline->highlight_callback;
} else if (colored_ccline->cmdfirstc == ':') {
try_enter(&tstate);
TRY_WRAP(&err, {
err_errmsg = N_("E5408: Unable to get g:Nvim_color_cmdline callback: %s");
dgc_ret = tv_dict_get_callback(&globvardict, S_LEN("Nvim_color_cmdline"),
&color_cb);
tl_ret = try_leave(&tstate, &err);
});
can_free_cb = true;
} else if (colored_ccline->cmdfirstc == '=') {
color_expr_cmdline(colored_ccline, ccline_colors);
}
if (!tl_ret || !dgc_ret) {
if (ERROR_SET(&err) || !dgc_ret) {
goto color_cmdline_error;
}
@ -3226,20 +3221,22 @@ static bool color_cmdline(CmdlineInfo *colored_ccline)
// correct, with msg_col it just misses leading `:`. Since `redraw!` in
// callback lags this is least of the user problems.
//
// Also using try_enter() because error messages may overwrite typed
// Also using TRY_WRAP because error messages may overwrite typed
// command-line which is not expected.
getln_interrupted_highlight = false;
try_enter(&tstate);
bool cbcall_ret = true;
TRY_WRAP(&err, {
err_errmsg = N_("E5407: Callback has thrown an exception: %s");
const int saved_msg_col = msg_col;
msg_silent++;
const bool cbcall_ret = callback_call(&color_cb, 1, &arg, &tv);
cbcall_ret = callback_call(&color_cb, 1, &arg, &tv);
msg_silent--;
msg_col = saved_msg_col;
if (got_int) {
getln_interrupted_highlight = true;
}
if (!try_leave(&tstate, &err) || !cbcall_ret) {
});
if (ERROR_SET(&err) || !cbcall_ret) {
goto color_cmdline_error;
}
if (tv.v_type != VAR_LIST) {

14
src/nvim/lua/stdlib.c
View File

@ -623,13 +623,10 @@ static int nlua_with(lua_State *L)
cmdmod.cmod_flags = flags;
apply_cmdmod(&cmdmod);
if (buf || win) {
try_start();
}
Error err = ERROR_INIT;
TRY_WRAP(&err, {
aco_save_T aco;
win_execute_T win_execute_args;
Error err = ERROR_INIT;
if (win) {
tabpage_T *tabpage = win_find_tabpage(win);
@ -650,11 +647,8 @@ static int nlua_with(lua_State *L)
} else if (buf) {
aucmd_restbuf(&aco);
}
end:
if (buf || win) {
try_end(&err);
}
end:;
});
undo_cmdmod(&cmdmod);
cmdmod = save_cmdmod;

2
src/nvim/option.c
View File

@ -3933,7 +3933,7 @@ static bool switch_option_context(void *const ctx, OptScope scope, void *const f
== FAIL) {
restore_win_noblock(switchwin, true);
if (try_end(err)) {
if (ERROR_SET(err)) {
return false;
}
api_set_error(err, kErrorTypeException, "Problem while switching windows");