diff options
author | Sean Dewar <seandewar@users.noreply.github.com> | 2022-08-30 23:13:52 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-30 23:13:52 +0100 |
commit | 813476bf7291dfaf9fc0ef77c9f53a07258a3801 (patch) | |
tree | d31ff4fa8f0b214f03172ec0473c1a22b8ed0139 | |
parent | 6b7eed1884c3b022cc15568c39f80f8f4da0780b (diff) | |
download | rneovim-813476bf7291dfaf9fc0ef77c9f53a07258a3801.tar.gz rneovim-813476bf7291dfaf9fc0ef77c9f53a07258a3801.tar.bz2 rneovim-813476bf7291dfaf9fc0ef77c9f53a07258a3801.zip |
fix(exceptions): restore `did_throw` (#20000)
`!did_throw` doesn't exactly imply `!current_exception`, as `did_throw = false`
is sometimes used to defer exception handling for later (without forgetting the
exception). E.g: uncaught exception handling in `do_cmdline()` may be deferred
to a different call (e.g: when `try_level > 0`).
In #7881, `current_exception = NULL` in `do_cmdline()` is used as an analogue of
`did_throw = false`, but also causes the pending exception to be lost, which
also leaks as `discard_exception()` wasn't used.
It may be possible to fix this by saving/restoring `current_exception`, but
handling all of `did_throw`'s edge cases seems messier. Maybe not worth
diverging over.
This fix also uncovers a `man_spec.lua` bug on Windows: exceptions are thrown
due to Windows missing `man`, but they're lost; skip these tests if `man` isn't
executable.
-rw-r--r-- | src/nvim/api/private/helpers.c | 8 | ||||
-rw-r--r-- | src/nvim/api/private/helpers.h | 1 | ||||
-rw-r--r-- | src/nvim/api/vimscript.c | 4 | ||||
-rw-r--r-- | src/nvim/eval.c | 2 | ||||
-rw-r--r-- | src/nvim/eval/userfunc.c | 2 | ||||
-rw-r--r-- | src/nvim/ex_docmd.c | 32 | ||||
-rw-r--r-- | src/nvim/ex_eval.c | 163 | ||||
-rw-r--r-- | src/nvim/globals.h | 6 | ||||
-rw-r--r-- | src/nvim/lua/executor.c | 2 | ||||
-rw-r--r-- | test/functional/plugin/man_spec.lua | 6 | ||||
-rw-r--r-- | test/functional/vimscript/eval_spec.lua | 28 |
11 files changed, 143 insertions, 111 deletions
diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c index e35c58bf1b..ebcf6cca6d 100644 --- a/src/nvim/api/private/helpers.c +++ b/src/nvim/api/private/helpers.c @@ -58,6 +58,7 @@ void try_enter(TryState *const tstate) .private_msg_list = NULL, .trylevel = trylevel, .got_int = got_int, + .did_throw = did_throw, .need_rethrow = need_rethrow, .did_emsg = did_emsg, }; @@ -65,6 +66,7 @@ void try_enter(TryState *const tstate) current_exception = NULL; trylevel = 1; got_int = false; + did_throw = false; need_rethrow = false; did_emsg = false; } @@ -85,6 +87,7 @@ bool try_leave(const TryState *const tstate, Error *const 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); @@ -93,6 +96,7 @@ bool try_leave(const TryState *const tstate, Error *const err) 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; @@ -127,7 +131,7 @@ bool try_end(Error *err) force_abort = false; if (got_int) { - if (current_exception) { + if (did_throw) { // If we got an interrupt, discard the current exception discard_current_exception(); } @@ -146,7 +150,7 @@ bool try_end(Error *err) if (should_free) { xfree(msg); } - } else if (current_exception) { + } else if (did_throw) { api_set_error(err, kErrorTypeException, "%s", current_exception->value); discard_current_exception(); } diff --git a/src/nvim/api/private/helpers.h b/src/nvim/api/private/helpers.h index 4608554448..2157ad0ec2 100644 --- a/src/nvim/api/private/helpers.h +++ b/src/nvim/api/private/helpers.h @@ -134,6 +134,7 @@ typedef struct { const msglist_T *const *msg_list; int trylevel; int got_int; + bool did_throw; int need_rethrow; int did_emsg; } TryState; diff --git a/src/nvim/api/vimscript.c b/src/nvim/api/vimscript.c index a28bfd2ab9..f6d0e39327 100644 --- a/src/nvim/api/vimscript.c +++ b/src/nvim/api/vimscript.c @@ -137,7 +137,7 @@ Object nvim_eval(String expr, Error *err) if (!recursive) { force_abort = false; suppress_errthrow = false; - current_exception = NULL; + did_throw = false; // `did_emsg` is set by emsg(), which cancels execution. did_emsg = false; } @@ -196,7 +196,7 @@ static Object _call_function(String fn, Array args, dict_T *self, Error *err) if (!recursive) { force_abort = false; suppress_errthrow = false; - current_exception = NULL; + did_throw = false; // `did_emsg` is set by emsg(), which cancels execution. did_emsg = false; } diff --git a/src/nvim/eval.c b/src/nvim/eval.c index 508d7cf491..2dbaa2f8ac 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -5985,7 +5985,7 @@ void timer_due_cb(TimeWatcher *tw, void *data) // Handle error message if (called_emsg > called_emsg_before && did_emsg) { timer->emsg_count++; - if (current_exception != NULL) { + if (did_throw) { discard_current_exception(); } } diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index b9a94a971b..1f94f9c2b7 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -3010,7 +3010,7 @@ void ex_call(exarg_T *eap) // When inside :try we need to check for following "| catch" or "| endtry". // Not when there was an error, but do check if an exception was thrown. - if ((!aborting() || current_exception != NULL) && (!failed || eap->cstack->cs_trylevel > 0)) { + if ((!aborting() || did_throw) && (!failed || eap->cstack->cs_trylevel > 0)) { // Check for trailing illegal characters and a following command. if (!ends_excmd(*arg)) { if (!failed && !aborting()) { diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index 1fa14f4e4f..bf518f3849 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -134,6 +134,7 @@ struct dbg_stuff { char *vv_throwpoint; int did_emsg; int got_int; + bool did_throw; int need_rethrow; int check_cstack; except_T *current_exception; @@ -165,6 +166,7 @@ static void save_dbg_stuff(struct dbg_stuff *dsp) // Necessary for debugging an inactive ":catch", ":finally", ":endtry". dsp->did_emsg = did_emsg; did_emsg = false; dsp->got_int = got_int; got_int = false; + dsp->did_throw = did_throw; did_throw = false; dsp->need_rethrow = need_rethrow; need_rethrow = false; dsp->check_cstack = check_cstack; check_cstack = false; dsp->current_exception = current_exception; current_exception = NULL; @@ -180,6 +182,7 @@ static void restore_dbg_stuff(struct dbg_stuff *dsp) (void)v_throwpoint(dsp->vv_throwpoint); did_emsg = dsp->did_emsg; got_int = dsp->got_int; + did_throw = dsp->did_throw; need_rethrow = dsp->need_rethrow; check_cstack = dsp->check_cstack; current_exception = dsp->current_exception; @@ -397,7 +400,8 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) initial_trylevel = trylevel; - current_exception = NULL; + // "did_throw" will be set to true when an exception is being thrown. + did_throw = false; // "did_emsg" will be set to true when emsg() is used, in which case we // cancel the whole command line, and any if/endif or loop. // If force_abort is set, we cancel everything. @@ -625,7 +629,7 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) // not to use a cs_line[] from an entry that isn't a ":while" // or ":for": It would make "current_line" invalid and can // cause a crash. - if (!did_emsg && !got_int && !current_exception + if (!did_emsg && !got_int && !did_throw && cstack.cs_idx >= 0 && (cstack.cs_flags[cstack.cs_idx] & (CSF_WHILE | CSF_FOR)) @@ -666,7 +670,7 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) current_line = 0; } - // A ":finally" makes did_emsg, got_int and current_exception pending for + // A ":finally" makes did_emsg, got_int and did_throw pending for // being restored at the ":endtry". Reset them here and set the // ACTIVE and FINALLY flags, so that the finally clause gets executed. // This includes the case where a missing ":endif", ":endwhile" or @@ -675,9 +679,8 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) cstack.cs_lflags &= ~CSL_HAD_FINA; report_make_pending((cstack.cs_pending[cstack.cs_idx] & (CSTP_ERROR | CSTP_INTERRUPT | CSTP_THROW)), - current_exception); - did_emsg = got_int = false; - current_exception = NULL; + did_throw ? current_exception : NULL); + did_emsg = got_int = did_throw = false; cstack.cs_flags[cstack.cs_idx] |= CSF_ACTIVE | CSF_FINALLY; } @@ -690,7 +693,7 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) // exception, cancel everything. If it is left normally, reset // force_abort to get the non-EH compatible abortion behavior for // the rest of the script. - if (trylevel == 0 && !did_emsg && !got_int && !current_exception) { + if (trylevel == 0 && !did_emsg && !got_int && !did_throw) { force_abort = false; } @@ -704,7 +707,7 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) // - didn't get an error message or lines are not typed // - there is a command after '|', inside a :if, :while, :for or :try, or // looping for ":source" command or function call. - } while (!((got_int || (did_emsg && force_abort) || current_exception) + } while (!((got_int || (did_emsg && force_abort) || did_throw) && cstack.cs_trylevel == 0) && !(did_emsg // Keep going when inside try/catch, so that the error can be @@ -724,7 +727,7 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) if (cstack.cs_idx >= 0) { // If a sourced file or executed function ran to its end, report the // unclosed conditional. - if (!got_int && !current_exception + if (!got_int && !did_throw && ((getline_equal(fgetline, cookie, getsourceline) && !source_finished(fgetline, cookie)) || (getline_equal(fgetline, cookie, get_func_line) @@ -767,7 +770,8 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) // conditional, discard the uncaught exception, disable the conversion // of interrupts or errors to exceptions, and ensure that no more // commands are executed. - if (current_exception) { + if (did_throw) { + assert(current_exception != NULL); char *p = NULL; msglist_T *messages = NULL; msglist_T *next; @@ -829,14 +833,14 @@ int do_cmdline(char *cmdline, LineGetter fgetline, void *cookie, int flags) // cstack belongs to the same function or, respectively, script file, it // will have to be checked for finally clauses to be executed due to the // ":return" or ":finish". This is done in do_one_cmd(). - if (current_exception) { + if (did_throw) { need_rethrow = true; } if ((getline_equal(fgetline, cookie, getsourceline) && ex_nesting_level > source_level(real_cookie)) || (getline_equal(fgetline, cookie, get_func_line) && ex_nesting_level > func_level(real_cookie) + 1)) { - if (!current_exception) { + if (!did_throw) { check_cstack = true; } } else { @@ -1675,7 +1679,7 @@ static void profile_cmd(const exarg_T *eap, cstack_T *cstack, LineGetter fgetlin && (!eap->skip || cstack->cs_idx == 0 || (cstack->cs_idx > 0 && (cstack->cs_flags[cstack->cs_idx - 1] & CSF_ACTIVE)))) { - int skip = did_emsg || got_int || current_exception; + bool skip = did_emsg || got_int || did_throw; if (eap->cmdidx == CMD_catch) { skip = !skip && !(cstack->cs_idx >= 0 @@ -1872,7 +1876,7 @@ static char *do_one_cmd(char **cmdlinep, int flags, cstack_T *cstack, LineGetter ea.skip = (did_emsg || got_int - || current_exception + || did_throw || (cstack->cs_idx >= 0 && !(cstack->cs_flags[cstack->cs_idx] & CSF_ACTIVE))); diff --git a/src/nvim/ex_eval.c b/src/nvim/ex_eval.c index 11795684cf..6dddafcfcb 100644 --- a/src/nvim/ex_eval.c +++ b/src/nvim/ex_eval.c @@ -49,16 +49,15 @@ // interrupt (got_int) under an active try conditional terminates the script // after the non-active finally clauses of all active try conditionals have been // executed. Otherwise, errors and/or interrupts are converted into catchable -// exceptions, which terminate the script only if not caught. For user -// exceptions, only current_exception is set. (Note: got_int can be set -// asynchronously afterwards by a SIGINT, so current_exception && got_int is not +// exceptions (did_throw additionally set), which terminate the script only if +// not caught. For user exceptions, only did_throw is set. (Note: got_int can +// be set asynchronously afterwards by a SIGINT, so did_throw && got_int is not // a reliant test that the exception currently being thrown is an interrupt // exception. Similarly, did_emsg can be set afterwards on an error in an -// (unskipped) conditional command inside an inactive conditional, so -// current_exception && did_emsg is not a reliant test that the exception -// currently being thrown is an error exception.) - The macros can be defined -// as expressions checking for a variable that is allowed to be changed during -// execution of a script. +// (unskipped) conditional command inside an inactive conditional, so did_throw +// && did_emsg is not a reliant test that the exception currently being thrown +// is an error exception.) - The macros can be defined as expressions checking +// for a variable that is allowed to be changed during execution of a script. // Values used for the Vim release. #define THROW_ON_ERROR true @@ -71,7 +70,7 @@ #define CHECK_SKIP \ (did_emsg \ || got_int \ - || current_exception \ + || did_throw \ || (cstack->cs_idx > 0 \ && !(cstack->cs_flags[cstack->cs_idx - 1] & CSF_ACTIVE))) @@ -104,7 +103,7 @@ static int cause_abort = false; /// value. "got_int" is also set by calling interrupt(). int aborting(void) { - return (did_emsg && force_abort) || got_int || current_exception; + return (did_emsg && force_abort) || got_int || did_throw; } /// The value of "force_abort" is temporarily reset by the first emsg() call @@ -186,7 +185,7 @@ bool cause_errthrow(const char *mesg, bool severe, bool *ignore) * currently throwing an exception, do nothing. The message text will * then be stored to v:errmsg by emsg() without displaying it. */ - if (((trylevel == 0 && !cause_abort) || emsg_silent) && !current_exception) { + if (((trylevel == 0 && !cause_abort) || emsg_silent) && !did_throw) { return false; } @@ -217,7 +216,7 @@ bool cause_errthrow(const char *mesg, bool severe, bool *ignore) * exception currently being thrown to prevent it from being caught. Just * execute finally clauses and terminate. */ - if (current_exception) { + if (did_throw) { // When discarding an interrupt exception, reset got_int to prevent the // same interrupt being converted to an exception again and discarding // the error exception we are about to throw here. @@ -349,7 +348,7 @@ int do_intthrow(cstack_T *cstack) { // If no interrupt occurred or no try conditional is active and no exception // is being thrown, do nothing (for compatibility of non-EH scripts). - if (!got_int || (trylevel == 0 && !current_exception)) { + if (!got_int || (trylevel == 0 && !did_throw)) { return false; } @@ -358,7 +357,7 @@ int do_intthrow(cstack_T *cstack) // The interrupt aborts everything except for executing finally clauses. // Discard any user or error or interrupt exception currently being // thrown. - if (current_exception) { + if (did_throw) { discard_current_exception(); } } else { @@ -369,7 +368,7 @@ int do_intthrow(cstack_T *cstack) // will be terminated then. - If an interrupt exception is already // being thrown, do nothing. - if (current_exception) { + if (did_throw) { if (current_exception->type == ET_INTERRUPT) { return false; } @@ -596,6 +595,7 @@ void discard_current_exception(void) } // Note: all globals manipulated here should be saved/restored in // try_enter/try_leave. + did_throw = false; need_rethrow = false; } @@ -1237,6 +1237,8 @@ void do_throw(cstack_T *cstack) cstack->cs_flags[idx] &= ~CSF_ACTIVE; cstack->cs_exception[idx] = current_exception; } + + did_throw = true; } /// Handle ":try" @@ -1345,7 +1347,7 @@ void ex_catch(exarg_T *eap) * corresponding try block never got active (because of an inactive * surrounding conditional or after an error or interrupt or throw). */ - if (!current_exception || !(cstack->cs_flags[idx] & CSF_TRUE)) { + if (!did_throw || !(cstack->cs_flags[idx] & CSF_TRUE)) { skip = true; } @@ -1406,10 +1408,10 @@ void ex_catch(exarg_T *eap) } if (caught) { - // Make this ":catch" clause active and reset did_emsg and got_int. - // Put the exception on the caught stack. + // Make this ":catch" clause active and reset did_emsg, got_int, + // and did_throw. Put the exception on the caught stack. cstack->cs_flags[idx] |= CSF_ACTIVE | CSF_CAUGHT; - did_emsg = got_int = false; + did_emsg = got_int = did_throw = false; catch_exception((except_T *)cstack->cs_exception[idx]); // It's mandatory that the current exception is stored in the cstack // so that it can be discarded at the next ":catch", ":finally", or @@ -1419,10 +1421,6 @@ void ex_catch(exarg_T *eap) if (cstack->cs_exception[cstack->cs_idx] != current_exception) { internal_error("ex_catch()"); } - // Discarding current_exceptions happens based on what is stored in - // cstack->cs_exception, *all* calls to discard_current_exception() are - // (and must be) guarded by current_exception check. - current_exception = NULL; } else { /* * If there is a preceding catch clause and it caught the exception, @@ -1477,14 +1475,12 @@ void ex_finally(exarg_T *eap) rewind_conditionals(cstack, idx, CSF_WHILE | CSF_FOR, &cstack->cs_looplevel); - /* - * Don't do something when the corresponding try block never got active - * (because of an inactive surrounding conditional or after an error or - * interrupt or throw) or for a ":finally" without ":try" or a multiple - * ":finally". After every other error (did_emsg or the conditional - * errors detected above) or after an interrupt (got_int) or an - * exception (current_exception), the finally clause must be executed. - */ + // Don't do something when the corresponding try block never got active + // (because of an inactive surrounding conditional or after an error or + // interrupt or throw) or for a ":finally" without ":try" or a multiple + // ":finally". After every other error (did_emsg or the conditional + // errors detected above) or after an interrupt (got_int) or an + // exception (did_throw), the finally clause must be executed. skip = !(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE); if (!skip) { @@ -1509,22 +1505,20 @@ void ex_finally(exarg_T *eap) */ cleanup_conditionals(cstack, CSF_TRY, false); - /* - * Make did_emsg, got_int, current_exception pending. If set, they - * overrule a pending ":continue", ":break", ":return", or ":finish". - * Then we have particularly to discard a pending return value (as done - * by the call to cleanup_conditionals() above when did_emsg or - * got_int is set). The pending values are restored by the - * ":endtry", except if there is a new error, interrupt, exception, - * ":continue", ":break", ":return", or ":finish" in the following - * finally clause. A missing ":endwhile", ":endfor" or ":endif" - * detected here is treated as if did_emsg and current_exception had - * already been set, respectively in case that the error is not - * converted to an exception, current_exception had already been unset. - * We must not set did_emsg here since that would suppress the - * error message. - */ - if (pending == CSTP_ERROR || did_emsg || got_int || current_exception) { + // Make did_emsg, got_int, did_throw pending. If set, they overrule + // a pending ":continue", ":break", ":return", or ":finish". Then + // we have particularly to discard a pending return value (as done + // by the call to cleanup_conditionals() above when did_emsg or + // got_int is set). The pending values are restored by the + // ":endtry", except if there is a new error, interrupt, exception, + // ":continue", ":break", ":return", or ":finish" in the following + // finally clause. A missing ":endwhile", ":endfor" or ":endif" + // detected here is treated as if did_emsg and did_throw had + // already been set, respectively in case that the error is not + // converted to an exception, did_throw had already been unset. + // We must not set did_emsg here since that would suppress the + // error message. + if (pending == CSTP_ERROR || did_emsg || got_int || did_throw) { if (cstack->cs_pending[cstack->cs_idx] == CSTP_RETURN) { report_discard_pending(CSTP_RETURN, cstack->cs_rettv[cstack->cs_idx]); @@ -1533,7 +1527,7 @@ void ex_finally(exarg_T *eap) if (pending == CSTP_ERROR && !did_emsg) { pending |= (THROW_ON_ERROR ? CSTP_THROW : 0); } else { - pending |= (current_exception ? CSTP_THROW : 0); + pending |= (did_throw ? CSTP_THROW : 0); } pending |= did_emsg ? CSTP_ERROR : 0; pending |= got_int ? CSTP_INTERRUPT : 0; @@ -1547,19 +1541,16 @@ void ex_finally(exarg_T *eap) // exception. When emsg() is called for a missing ":endif" or // a missing ":endwhile"/":endfor" detected here, the // exception will be discarded. - if (current_exception - && cstack->cs_exception[cstack->cs_idx] != current_exception) { + if (did_throw && cstack->cs_exception[cstack->cs_idx] != current_exception) { internal_error("ex_finally()"); } } - /* - * Set CSL_HAD_FINA, so do_cmdline() will reset did_emsg, - * got_int, and current_exception and make the finally clause active. - * This will happen after emsg() has been called for a missing - * ":endif" or a missing ":endwhile"/":endfor" detected here, so - * that the following finally clause will be executed even then. - */ + // Set CSL_HAD_FINA, so do_cmdline() will reset did_emsg, + // got_int, and did_throw and make the finally clause active. + // This will happen after emsg() has been called for a missing + // ":endif" or a missing ":endwhile"/":endfor" detected here, so + // that the following finally clause will be executed even then. cstack->cs_lflags |= CSL_HAD_FINA; } } @@ -1587,8 +1578,7 @@ void ex_endtry(exarg_T *eap) // made inactive by a ":continue", ":break", ":return", or ":finish" in // the finally clause. The latter case need not be tested since then // anything pending has already been discarded. - bool skip = did_emsg || got_int || current_exception - || !(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE); + bool skip = did_emsg || got_int || did_throw || !(cstack->cs_flags[cstack->cs_idx] & CSF_TRUE); if (!(cstack->cs_flags[cstack->cs_idx] & CSF_TRY)) { eap->errmsg = get_end_emsg(cstack); @@ -1602,14 +1592,12 @@ void ex_endtry(exarg_T *eap) &cstack->cs_looplevel); skip = true; - /* - * If an exception is being thrown, discard it to prevent it from - * being rethrown at the end of this function. It would be - * discarded by the error message, anyway. Resets current_exception. - * This does not affect the script termination due to the error - * since "trylevel" is decremented after emsg() has been called. - */ - if (current_exception) { + // If an exception is being thrown, discard it to prevent it from + // being rethrown at the end of this function. It would be + // discarded by the error message, anyway. Resets did_throw. + // This does not affect the script termination due to the error + // since "trylevel" is decremented after emsg() has been called. + if (did_throw) { discard_current_exception(); } @@ -1624,7 +1612,7 @@ void ex_endtry(exarg_T *eap) * a finally clause, we need to rethrow it after closing the try * conditional. */ - if (current_exception + if (did_throw && (cstack->cs_flags[idx] & CSF_TRUE) && !(cstack->cs_flags[idx] & CSF_FINALLY)) { rethrow = true; @@ -1648,10 +1636,10 @@ void ex_endtry(exarg_T *eap) if (got_int) { skip = true; (void)do_intthrow(cstack); - // The do_intthrow() call may have reset current_exception or + // The do_intthrow() call may have reset did_throw or // cstack->cs_pending[idx]. rethrow = false; - if (current_exception && !(cstack->cs_flags[idx] & CSF_FINALLY)) { + if (did_throw && !(cstack->cs_flags[idx] & CSF_FINALLY)) { rethrow = true; } } @@ -1721,7 +1709,7 @@ void ex_endtry(exarg_T *eap) // When the finally clause was entered due to an error, // interrupt or throw (as opposed to a ":continue", ":break", // ":return", or ":finish"), restore the pending values of - // did_emsg, got_int, and current_exception. This is skipped, if there + // did_emsg, got_int, and did_throw. This is skipped, if there // was a new error, interrupt, throw, ":continue", ":break", // ":return", or ":finish". in the finally clause. default: @@ -1768,24 +1756,22 @@ void enter_cleanup(cleanup_T *csp) { int pending = CSTP_NONE; - /* - * Postpone did_emsg, got_int, current_exception. The pending values will be - * restored by leave_cleanup() except if there was an aborting error, - * interrupt, or uncaught exception after this function ends. - */ - if (did_emsg || got_int || current_exception || need_rethrow) { + // Postpone did_emsg, got_int, did_throw. The pending values will be + // restored by leave_cleanup() except if there was an aborting error, + // interrupt, or uncaught exception after this function ends. + if (did_emsg || got_int || did_throw || need_rethrow) { csp->pending = (did_emsg ? CSTP_ERROR : 0) | (got_int ? CSTP_INTERRUPT : 0) - | (current_exception ? CSTP_THROW : 0) + | (did_throw ? CSTP_THROW : 0) | (need_rethrow ? CSTP_THROW : 0); - // If we are currently throwing an exception, save it as well. On an error - // not yet converted to an exception, update "force_abort" and reset - // "cause_abort" (as do_errthrow() would do). This is needed for the - // do_cmdline() call that is going to be made for autocommand execution. We - // need not save *msg_list because there is an extra instance for every call - // of do_cmdline(), anyway. - if (current_exception || need_rethrow) { + // If we are currently throwing an exception (did_throw), save it as + // well. On an error not yet converted to an exception, update + // "force_abort" and reset "cause_abort" (as do_errthrow() would do). + // This is needed for the do_cmdline() call that is going to be made + // for autocommand execution. We need not save *msg_list because + // there is an extra instance for every call of do_cmdline(), anyway. + if (did_throw || need_rethrow) { csp->exception = current_exception; current_exception = NULL; } else { @@ -1795,8 +1781,7 @@ void enter_cleanup(cleanup_T *csp) cause_abort = false; } } - did_emsg = got_int = need_rethrow = false; - current_exception = NULL; + did_emsg = got_int = did_throw = need_rethrow = false; // Report if required by the 'verbose' option or when debugging. report_make_pending(pending, csp->exception); @@ -1866,7 +1851,7 @@ void leave_cleanup(cleanup_T *csp) force_abort = false; } - // Restore the pending values of did_emsg, got_int, and current_exception. + // Restore the pending values of did_emsg, got_int, and did_throw. if (pending & CSTP_ERROR) { did_emsg = true; } @@ -1874,7 +1859,7 @@ void leave_cleanup(cleanup_T *csp) got_int = true; } if (pending & CSTP_THROW) { - need_rethrow = true; // current_exception will be set by do_one_cmd() + need_rethrow = true; // did_throw will be set by do_one_cmd() } // Report if required by the 'verbose' option or when debugging. diff --git a/src/nvim/globals.h b/src/nvim/globals.h index ddbe727db9..060db3b3b1 100644 --- a/src/nvim/globals.h +++ b/src/nvim/globals.h @@ -241,9 +241,13 @@ EXTERN int do_profiling INIT(= PROF_NONE); ///< PROF_ values /// Exception currently being thrown. Used to pass an exception to a different /// cstack. Also used for discarding an exception before it is caught or made -/// pending. +/// pending. Only valid when did_throw is true. EXTERN except_T *current_exception; +/// An exception is being thrown. Reset when the exception is caught or as +/// long as it is pending in a finally clause. +EXTERN bool did_throw INIT(= false); + /// Set when a throw that cannot be handled in do_cmdline() must be propagated /// to the cstack of the previously called do_cmdline(). EXTERN bool need_rethrow INIT(= false); diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c index 39585ac182..42aa13cfc1 100644 --- a/src/nvim/lua/executor.c +++ b/src/nvim/lua/executor.c @@ -1027,7 +1027,7 @@ int nlua_call(lua_State *lstate) // TODO(bfredl): this should be simplified in error handling refactor force_abort = false; suppress_errthrow = false; - current_exception = NULL; + did_throw = false; did_emsg = false; try_start(); diff --git a/test/functional/plugin/man_spec.lua b/test/functional/plugin/man_spec.lua index c8da5a711f..9304aa6da9 100644 --- a/test/functional/plugin/man_spec.lua +++ b/test/functional/plugin/man_spec.lua @@ -6,6 +6,12 @@ local funcs = helpers.funcs local nvim_prog = helpers.nvim_prog local matches = helpers.matches +clear() +if funcs.executable('man') == 0 then + pending('missing "man" command', function() end) + return +end + describe(':Man', function() before_each(function() clear() diff --git a/test/functional/vimscript/eval_spec.lua b/test/functional/vimscript/eval_spec.lua index 65ac5b8c80..d4fa7afe89 100644 --- a/test/functional/vimscript/eval_spec.lua +++ b/test/functional/vimscript/eval_spec.lua @@ -219,3 +219,31 @@ it('Error when if/for/while/try/function is nested too deep',function() feed(':call Test5()<CR>') screen:expect({any = 'E1058: '}) end) + +describe("uncaught exception", function() + before_each(clear) + after_each(function() + os.remove('throw1.vim') + os.remove('throw2.vim') + os.remove('throw3.vim') + end) + + it('is not forgotten #13490', function() + command('autocmd BufWinEnter * throw "i am error"') + eq('i am error', exc_exec('try | new | endtry')) + + -- Like Vim, throwing here aborts the processing of the script, but does not stop :runtime! + -- from processing the others. + -- Only the first thrown exception should be rethrown from the :try below, though. + for i = 1, 3 do + write_file('throw' .. i .. '.vim', ([[ + let result ..= '%d' + throw 'throw%d' + let result ..= 'X' + ]]):format(i, i)) + end + command('set runtimepath+=. | let result = ""') + eq('throw1', exc_exec('try | runtime! throw*.vim | endtry')) + eq('123', eval('result')) + end) +end) |