Re: err_exit/err_return regression

[Bart Schaefer <schaefer@brasslantern.com>]

On Sep 29, 10:52pm, Joshua Krusell wrote:
}
} Was in a hurry and didn't provide much context, but that was with zsh
} 5.1.1. I just checked out master and I'm still getting the same results
} on both OSX and Linux.

I can confirm.  If you also "setopt xtrace" you can see that it returns
at [[ -n '' ]].  If there is anything other than an assignment in the
"then" part of that "if", the bug does not occur, so I think this may
be in part a parsing/wordcode problem.

In the case with the assignment, when we hit line 1209 of exec.c [in
execlist()] the test (ltype & Z_ZIMPLE) is true, and eventually we
hit the same test at line 1252 and "goto sublist_done" which [here I
begin to lose track of the exact sequence of events] skips over the
execpline() call on line 1284 and therefore never resets lastval = 0
at line 569 of execif(), so we obey errreturn.

If it's a command rather than an assignment, we go through the
WC_SUBLIST branch instead, and everything works out.

I think what this boils down to is, "retflag" needs two values to
distinguish an actual return from an ERR_RETURN, the same way that
we distinguish interrupts from actual errors.  Patch below seems to
work for the "if" case, there may be other such cases.  Might want
to use bitflags and/or #defines instead of just 1 and 2.

Or maybe there's some other way to untangle the Z_SIMPLE case in the
exec*() chain and the patch below isn't needed at all.

Here's a cut-and-paste-able version of the failing example (before
the patch).  Replace "a=2" with e.g. "a=2 :" to compare.

source =(<<\EOF
setopt err_return xtrace

print $ZSH_VERSION
setopt

if false; then
    :
else
    if [[ -n '' ]]; then
        a=2  
    fi

    print foo
fi
EOF
)


diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 97c3370..691aaeb 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -175,7 +175,7 @@ pattern are loaded.
 With the tt(-w) flag, the var(name)s are taken as names of files compiled
 with the tt(zcompile) builtin, and all functions defined in them are
 marked for autoloading.  Note this does not otherwise change the search
-order for 
+order via tt(fpath) when the function is first called.
 
 The flags tt(-z) and tt(-k) mark the function to be autoloaded using the
 zsh or ksh style, as if the option tt(KSH_AUTOLOAD) were unset or were
diff --git a/Src/exec.c b/Src/exec.c
index da808d6..154bbb8 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1408,7 +1408,7 @@ sublist_done:
 			exit(lastval);
 		}
 		if (errreturn) {
-		    retflag = 1;
+		    retflag = 2;
 		    breaks = loops;
 		}
 	    }
diff --git a/Src/loop.c b/Src/loop.c
index 4def9b6..7d1528e 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -552,8 +552,12 @@ execif(Estate state, int do_exec)
 	    run = 1;
 	    break;
 	}
-	if (retflag)
-	    break;
+	if (retflag) {
+	    if (retflag == 2)
+		retflag = 0; /* Never ERR_RETURN here */
+	    else
+		break;
+	}
 	s = 1;
 	state->pc = next;
     }