* Question about err_return
@ 2017-08-21 5:28 Radon Rosborough
2017-08-21 6:27 ` Daniel Shahaf
2017-08-23 16:56 ` Peter Stephenson
0 siblings, 2 replies; 5+ messages in thread
From: Radon Rosborough @ 2017-08-21 5:28 UTC (permalink / raw)
To: zsh-workers
Hi all,
I have a question about err_return. Specifically, why does the
following:
function { setopt err_return; false; echo 'oh no' }
print nothing, while
function { setopt err_return; false; echo 'oh no' } && true
prints 'oh no'? This seems very inconsistent to me, as I would expect
the result of 'x && true' to be the same as 'x' in all circumstances.
Now I found an old thread [1] about this, and the resolution was that
this behavior was unsurprising since err_return emulates err_exit. The
idea is that err_exit is automatically disabled for all child code
whenever a function is invoked as part of a conditional expression,
and so err_return does the same.
And I also found an old thread [2] on the Bash mailing list, which was
a proposal to add an err_return to Bash that would act in the way that
I expect (namely, that both examples up above would print nothing).
One of the cited advantages was that while err_exit couldn't be
changed due to backwards compatibility, its "broken" behavior could be
mitigated by a new err_return option that would work better. AFAICT,
this proposal went nowhere.
But we have an err_return in Zsh. Is it worth breaking backward
compatibility to improve the usability of err_return?
If not, I would like to know how I can otherwise implement error
checking in my scripts. I need for any unexpected error to cause an
immediate return from the enclosing function, which is how err_return
is advertised in the Zsh manual [3], but not how it actually works at
present. And preferably I would like to implement this error checking
without suffixing N hundred lines of code with '|| return $?'.
Best,
Radon Rosborough
[1]: https://www.zsh.org/mla/users/2012/msg00813.html
[2]: https://lists.gnu.org/archive/html/bug-bash/2010-05/msg00164.html
[3]: http://zsh.sourceforge.net/Doc/Release/Options.html#Scripts-and-Functions
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Question about err_return
2017-08-21 5:28 Question about err_return Radon Rosborough
@ 2017-08-21 6:27 ` Daniel Shahaf
2017-08-23 16:56 ` Peter Stephenson
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Shahaf @ 2017-08-21 6:27 UTC (permalink / raw)
To: Radon Rosborough, zsh-workers
Radon Rosborough wrote on Sun, 20 Aug 2017 22:28 -0700:
> why does the following:
>
> function { setopt err_return; false; echo 'oh no' }
>
> print nothing, while
>
> function { setopt err_return; false; echo 'oh no' } && true
>
> prints 'oh no'? This seems very inconsistent to me, as I would expect
> the result of 'x && true' to be the same as 'x' in all circumstances.
>
> Now I found an old thread [1] about this, and the resolution was that
> this behavior was unsurprising since err_return emulates err_exit. The
> idea is that err_exit is automatically disabled for all child code
> whenever a function is invoked as part of a conditional expression,
> and so err_return does the same.
I'm having trouble seeing how this is not simply a bug. Consider:
.
% f() { setopt err_return; false; echo 'this runs' }
% f && :
this runs
The command "false" had a non-zero exit status, and the enclosing function
continued to execute. That's exactly what the documentation promises would
not happen:
.
ERR_RETURN
If a command has a non-zero exit status, return immediately from
the enclosing function.
I don't see how the fact that f's callsite is the left-hand side of a '&&'
is relevant here. It would be relevant to deciding whether f's non-zero
return value should cause the _caller_ to return/exit immediately as well,
but that has to do with the ERR_EXIT / ERR_RETURN settings in the caller
scope, and shouldn't change f's behaviour. (The invariant you proposed —
«foo» ≡ «foo && true» — is a good one.)
The analogy with ERR_EXIT's treatment of '&&'/'||' should kick in when those
oprators are used _inside_ the function:
.
% f() { setopt err_return; false || true; echo 'this should be printed' }
The callee function behaves the same way if err_return is set in the caller, as
Bart suggested in the original thread.
> But we have an err_return in Zsh. Is it worth breaking backward
> compatibility to improve the usability of err_return?
Well, I'm certainly not suggesting to change this before 5.4.2 (which is
expected soon). But we could change this after 5.4.2 and let it soak in master
until 5.5. Thoughts?
> If not, I would like to know how I can otherwise implement error
> checking in my scripts. I need for any unexpected error to cause an
> immediate return from the enclosing function, which is how err_return
> is advertised in the Zsh manual [3], but not how it actually works at
> present.
Yes, the documentation doesn't match the implementation, so we need to fix one
or the other.
Thanks for the report, and for digging up those old threads.
Cheers,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Question about err_return
2017-08-21 5:28 Question about err_return Radon Rosborough
2017-08-21 6:27 ` Daniel Shahaf
@ 2017-08-23 16:56 ` Peter Stephenson
2017-08-23 20:09 ` Peter Stephenson
1 sibling, 1 reply; 5+ messages in thread
From: Peter Stephenson @ 2017-08-23 16:56 UTC (permalink / raw)
To: Radon Rosborough, zsh-workers
On Sun, 20 Aug 2017 22:28:34 -0700
Radon Rosborough <radon.neon@gmail.com> wrote:
> Hi all,
>
> I have a question about err_return. Specifically, why does the
> following:
>
> function { setopt err_return; false; echo 'oh no' }
>
> print nothing, while
>
> function { setopt err_return; false; echo 'oh no' } && true
>
> prints 'oh no'? This seems very inconsistent to me, as I would expect
> the result of 'x && true' to be the same as 'x' in all circumstances.
It's probably just a bug that we don't reset the state inside functions.
We did have tests assuming it worked this way, but it's not documented
and not obvious, so I think it makes sense to change it.
I can do this, but I need to be careful about one subtlety: whether
ERR_EXIT is still suppressed in that case. I think the answer is yes
--- it applies globally rather than hierarchically. To go with bash, it
would mean (so far as I can see) this applies even if err_exit is
explicitly set in the function --- i.e. suppression of the effect really
is controlled by internal and not user-visible state.
In that case I'll have to do a bit more work separating out the innards
of the two options.
pws
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Question about err_return
2017-08-23 16:56 ` Peter Stephenson
@ 2017-08-23 20:09 ` Peter Stephenson
2017-08-24 10:33 ` Peter Stephenson
0 siblings, 1 reply; 5+ messages in thread
From: Peter Stephenson @ 2017-08-23 20:09 UTC (permalink / raw)
To: zsh-workers
On Wed, 23 Aug 2017 17:56:08 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> I can do this, but I need to be careful about one subtlety: whether
> ERR_EXIT is still suppressed in that case. I think the answer is yes
> --- it applies globally rather than hierarchically. To go with bash, it
> would mean (so far as I can see) this applies even if err_exit is
> explicitly set in the function --- i.e. suppression of the effect really
> is controlled by internal and not user-visible state.
I think this does the trick. It's not entirely trivial
[traditional pause for gasps of amazement]
but the logic associated with noerrexit should be a bit more readable
now.
pws
diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 70092d6..42571fc 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1659,6 +1659,13 @@ as it is by default, and the option tt(ERR_EXIT) is found to have been set
on exit, then the command for which the tt(DEBUG) trap is being executed is
skipped. The option is restored after the trap exits.
+Non-zero status in a command list containing tt(&&) or tt(||) is ignored
+for commands not at the end of the list. Hence
+
+example(false && true)
+
+does not trigger exit.
+
Exiting due to tt(ERR_EXIT) has certain interactions with asynchronous
jobs noted in
ifzman(the section JOBS in zmanref(zshmisc))\
@@ -1672,10 +1679,25 @@ cindex(function return, on error)
cindex(return from function, on error)
item(tt(ERR_RETURN))(
If a command has a non-zero exit status, return immediately from the
-enclosing function. The logic is identical to that for tt(ERR_EXIT),
+enclosing function. The logic is similar to that for tt(ERR_EXIT),
except that an implicit tt(return) statement is executed instead of an
tt(exit). This will trigger an exit at the outermost level of a
non-interactive script.
+
+Normally this option inherits the behaviour of tt(ERR_EXIT) that
+code followed by `tt(&&)' `tt(||)' does not trigger a return. Hence
+in the following:
+
+example(summit || true)
+
+no return is forced as the combined effect always has a zero return
+status.
+
+Note. however, that if tt(summit) in the above example is itself a
+function, code inside it is considered separately: it may force a return
+from tt(summit) (assuming the option remains set within tt(summit)), but
+not from the enclosing context. This behaviour is different from
+tt(ERR_EXIT) which is unaffected by function scope.
)
pindex(EVAL_LINENO)
pindex(NO_EVAL_LINENO)
diff --git a/README b/README
index f711b17..d59ddfe 100644
--- a/README
+++ b/README
@@ -81,6 +81,19 @@ against user defined widgets inadvertently reading from the tty device,
and addresses the antisocial behaviour of running a command with its
stdin closed.
+5) [New between 5.4.1 and 5.4.2] In previous versions of the shell, the
+following code:
+
+ () { setopt err_return; false; echo 'oh no' } && true
+
+printed "oh no", as the ERR_RETURN behaviour was suppressed when
+a function was executed on the left hand side of an "&&" list. This was
+undocumented and inconvenient as it is generally more useful to consider
+execution within a function in isolation from its environment. The shell
+now returns from the function on executing `false'. (This is general
+to all functions; an anonymous function is shown here for compactness.)
+
+
Incompatibilities between 5.0.8 and 5.3
----------------------------------------
diff --git a/Src/exec.c b/Src/exec.c
index 9996dff..cd99733 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -41,12 +41,15 @@ enum {
ADDVAR_RESTORE = 1 << 2
};
-/* used to suppress ERREXIT and trapping of SIGZERR, SIGEXIT. */
+/*
+ * used to suppress ERREXIT and trapping of SIGZERR, SIGEXIT.
+ * Bits from noerrexit_bits.
+ */
/**/
int noerrexit;
-/* used to suppress ERREXIT for one occurrence */
+/* used to suppress ERREXIT or ERRRETURN for one occurrence: 0 or 1 */
/**/
int this_noerrexit;
@@ -1299,7 +1302,7 @@ execlist(Estate state, int dont_change_job, int exiting)
int oerrexit_opt = opts[ERREXIT];
Param pm;
opts[ERREXIT] = 0;
- noerrexit = 1;
+ noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
if (ltype & Z_SIMPLE) /* skip the line number */
pc2++;
pm = setsparam("ZSH_DEBUG_CMD", getpermtext(state->prog, pc2, 0));
@@ -1351,13 +1354,13 @@ execlist(Estate state, int dont_change_job, int exiting)
int isend = (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END);
next = state->pc + WC_SUBLIST_SKIP(code);
if (!oldnoerrexit)
- noerrexit = !isend;
+ noerrexit = isend ? 0 : NOERREXIT_EXIT | NOERREXIT_RETURN;
if (WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT) {
/* suppress errexit for "! this_command" */
if (isend)
this_noerrexit = 1;
/* suppress errexit for ! <list-of-shell-commands> */
- noerrexit = 1;
+ noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
}
switch (WC_SUBLIST_TYPE(code)) {
case WC_SUBLIST_END:
@@ -1444,11 +1447,11 @@ sublist_done:
/*
* See hairy code near the end of execif() for the
- * following. "noerrexit == 2" only applies until
+ * following. "noerrexit " only applies until
* we hit execcmd on the way down. We're now
* on the way back up, so don't restore it.
*/
- if (oldnoerrexit != 2)
+ if (!(oldnoerrexit & NOERREXIT_UNTIL_EXEC))
noerrexit = oldnoerrexit;
if (sigtrapped[SIGDEBUG] && !isset(DEBUGBEFORECMD) && !donedebug) {
@@ -1458,7 +1461,7 @@ sublist_done:
*/
int oerrexit_opt = opts[ERREXIT];
opts[ERREXIT] = 0;
- noerrexit = 1;
+ noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
exiting = donetrap;
ret = lastval;
dotrap(SIGDEBUG);
@@ -1474,16 +1477,19 @@ sublist_done:
/* Check whether we are suppressing traps/errexit *
* (typically in init scripts) and if we haven't *
* already performed them for this sublist. */
- if (!noerrexit && !this_noerrexit && !donetrap && !this_donetrap) {
- if (sigtrapped[SIGZERR] && lastval) {
+ if (!this_noerrexit && !donetrap && !this_donetrap) {
+ if (sigtrapped[SIGZERR] && lastval &&
+ !(noerrexit & NOERREXIT_EXIT)) {
dotrap(SIGZERR);
donetrap = 1;
}
if (lastval) {
int errreturn = isset(ERRRETURN) &&
- (isset(INTERACTIVE) || locallevel || sourcelevel);
- int errexit = isset(ERREXIT) ||
- (isset(ERRRETURN) && !errreturn);
+ (isset(INTERACTIVE) || locallevel || sourcelevel) &&
+ !(noerrexit & NOERREXIT_RETURN);
+ int errexit = (isset(ERREXIT) ||
+ (isset(ERRRETURN) && !errreturn)) &&
+ !(noerrexit & NOERREXIT_EXIT);
if (errexit) {
if (sigtrapped[SIGEXIT])
dotrap(SIGEXIT);
@@ -3019,7 +3025,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
preargs = NULL;
/* if we get this far, it is OK to pay attention to lastval again */
- if (noerrexit == 2 && !is_shfunc)
+ if ((noerrexit & NOERREXIT_UNTIL_EXEC) && !is_shfunc)
noerrexit = 0;
/* Do prefork substitutions.
@@ -5462,6 +5468,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
char saveopts[OPT_SIZE], *oldscriptname = scriptname;
char *name = shfunc->node.nam;
int flags = shfunc->node.flags, ooflags;
+ int savnoerrexit;
char *fname = dupstring(name);
int obreaks, ocontflag, oloops, saveemulation, restore_sticky;
Eprog prog;
@@ -5484,6 +5491,11 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
trap_return--;
oldlastval = lastval;
oldnumpipestats = numpipestats;
+ savnoerrexit = noerrexit;
+ /*
+ * Suppression of ERR_RETURN is turned off in function scope.
+ */
+ noerrexit &= ~NOERREXIT_RETURN;
if (noreturnval) {
/*
* Easiest to use the heap here since we're bracketed
@@ -5702,6 +5714,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
if (trap_state == TRAP_STATE_PRIMED)
trap_return++;
ret = lastval;
+ noerrexit = savnoerrexit;
if (noreturnval) {
lastval = oldlastval;
numpipestats = oldnumpipestats;
diff --git a/Src/init.c b/Src/init.c
index d8c26ac..9331b03 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1070,7 +1070,7 @@ setupvals(char *cmd, char *runscript, char *zsh_name)
sfcontext = SFC_NONE;
trap_return = 0;
trap_state = TRAP_STATE_INACTIVE;
- noerrexit = -1;
+ noerrexit = NOERREXIT_SIGNAL;
nohistsave = 1;
dirstack = znewlinklist();
bufstack = znewlinklist();
@@ -1199,7 +1199,7 @@ init_signals(void)
void
run_init_scripts(void)
{
- noerrexit = -1;
+ noerrexit = NOERREXIT_SIGNAL;
if (EMULATION(EMULATE_KSH|EMULATE_SH)) {
if (islogin)
diff --git a/Src/loop.c b/Src/loop.c
index f7eae30..4859c97 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -542,7 +542,7 @@ execif(Estate state, int do_exec)
end = state->pc + WC_IF_SKIP(code);
if (!noerrexit)
- noerrexit = 1;
+ noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
while (state->pc < end) {
code = *state->pc++;
if (wc_code(code) != WC_IF ||
@@ -567,7 +567,12 @@ execif(Estate state, int do_exec)
if (run) {
/* we need to ignore lastval until we reach execcmd() */
- noerrexit = olderrexit ? olderrexit : lastval ? 2 : 0;
+ if (olderrexit)
+ noerrexit = olderrexit;
+ else if (lastval)
+ noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN | NOERREXIT_UNTIL_EXEC;
+ else
+ noerrexit = 0;
cmdpush(run == 2 ? CS_ELSE : (s ? CS_ELIFTHEN : CS_IFTHEN));
execlist(state, 1, do_exec);
cmdpop();
diff --git a/Src/signals.c b/Src/signals.c
index cad40f4..94f379e 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -652,7 +652,7 @@ zhandler(int sig)
case SIGINT:
if (!handletrap(SIGINT)) {
if ((isset(PRIVILEGED) || isset(RESTRICTED)) &&
- isset(INTERACTIVE) && noerrexit < 0)
+ isset(INTERACTIVE) && (noerrexit & NOERREXIT_SIGNAL))
zexit(SIGINT, 1);
if (list_pipe || chline || simple_pline) {
breaks = loops;
diff --git a/Src/zsh.h b/Src/zsh.h
index 91e8d7f..abe9a9c 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2122,6 +2122,17 @@ enum source_return {
SOURCE_ERROR = 2
};
+enum noerrexit_bits {
+ /* Suppress ERR_EXIT and traps: global */
+ NOERREXIT_EXIT = 1,
+ /* Suppress ERR_RETURN: per function call */
+ NOERREXIT_RETURN = 2,
+ /* NOERREXIT only needed on way down */
+ NOERREXIT_UNTIL_EXEC = 4,
+ /* Force exit on SIGINT */
+ NOERREXIT_SIGNAL = 8
+};
+
/***********************************/
/* Definitions for history control */
/***********************************/
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index f01d835..8101ff5 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -345,6 +345,36 @@
>ZERR trapped
>off after
+ (
+ setopt ERR_EXIT
+ () { false; print is executed; } && true
+ () { false; print is not executed; }
+ print is not executed
+ )
+1:ERR_EXIT is suppressed within a function followed by "&&"
+>is executed
+
+ (
+ setopt ERR_RETURN
+ () { false; print is not executed; } || print is executed
+ print is also executed
+ )
+0:ERR_RETURN is not suppressed within a function followed by "||"
+>is executed
+>is also executed
+
+ (
+ setopt ERR_RETURN
+ () {
+ () { false; print Not executed 1; } || true
+ print Executed
+ () { false; print Not executed 2; }
+ print Not executed 3;
+ } && false
+ )
+1:ERR_RETURN with additional levels
+>Executed
+
(print before; setopt noexec; print after)
0:NO_EXEC option
>before
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Question about err_return
2017-08-23 20:09 ` Peter Stephenson
@ 2017-08-24 10:33 ` Peter Stephenson
0 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2017-08-24 10:33 UTC (permalink / raw)
To: zsh-workers
On Wed, 23 Aug 2017 21:09:51 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> diff --git a/Src/init.c b/Src/init.c
> index d8c26ac..9331b03 100644
> --- a/Src/init.c
> +++ b/Src/init.c
> @@ -1070,7 +1070,7 @@ setupvals(char *cmd, char *runscript, char *zsh_name)
> sfcontext = SFC_NONE;
> trap_return = 0;
> trap_state = TRAP_STATE_INACTIVE;
> - noerrexit = -1;
> + noerrexit = NOERREXIT_SIGNAL;
> nohistsave = 1;
> dirstack = znewlinklist();
> bufstack = znewlinklist();
> @@ -1199,7 +1199,7 @@ init_signals(void)
> void
> run_init_scripts(void)
> {
> - noerrexit = -1;
> + noerrexit = NOERREXIT_SIGNAL;
>
> if (EMULATION(EMULATE_KSH|EMULATE_SH)) {
> if (islogin)
I think the two assignments modified here need the standard flags as
well since the intention is obviously to suppress error exit/return
generally in startup files (not just modify the ^C behaviour, which is
what the flag there at the moment does).
Obviously err_return in functions called from initialisation files
would get the new behaviour. That's probably sensible if your
functions are designed to operate with the option. It's a little
less obvious with anonymous functions, perhaps.
Apart from that, I hope this isn't going to be problematic...
pws
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-24 10:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-21 5:28 Question about err_return Radon Rosborough
2017-08-21 6:27 ` Daniel Shahaf
2017-08-23 16:56 ` Peter Stephenson
2017-08-23 20:09 ` Peter Stephenson
2017-08-24 10:33 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).