err_return in initialization scripts returns from conditionals

err_return in initialization scripts returns from conditionals

[Jan Alexander Steffens]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

Hi list,

Since recently (5.4.2?), there seems to be a bug involving err_return
during initialization.

If I have a .zshrc with the following content:

    setopt err_return
    true && function {
      if false; then :; fi
      echo 1
    }
    echo 2

Then the "if false" causes the function to return and "1" is not printed.
Note that the behavior differs from running this file using "zsh .zshrc",
where both lines are printed, which I expect is the intended behavior.

Greetings,
Jan Steffens

Re: err_return in initialization scripts returns from conditionals

[Peter Stephenson]

On Wed, 30 Aug 2017 20:36:48 +0000
Jan Alexander Steffens <jan.steffens@gmail.com> wrote:
> Since recently (5.4.2?), there seems to be a bug involving err_return
> during initialization.
> 
> If I have a .zshrc with the following content:
> 
>     setopt err_return
>     true && function {
>       if false; then :; fi
>       echo 1
>     }
>     echo 2
> 
> Then the "if false" causes the function to return and "1" is not printed.
> Note that the behavior differs from running this file using "zsh .zshrc",
> where both lines are printed, which I expect is the intended behavior.

Yes, err_return (and err_exit which isn't affected by this problem) are
supposed to be suppressed at that point in initialisation as in normal
code.  This shows up because initialisation is already specially handled
and this was complicating the check for the state at that point.

It seems that the extra special tweaks for the tweaks to support the
option need some extra extra special tweaks.

pws

diff --git a/Src/loop.c b/Src/loop.c
index 4859c97..7cd5219 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -541,8 +541,9 @@ execif(Estate state, int do_exec)
     olderrexit = noerrexit;
     end = state->pc + WC_IF_SKIP(code);
 
-    if (!noerrexit)
-	noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
+    /* Preserve special handling for init code */
+    if (!noerrexit || (noerrexit & NOERREXIT_SIGNAL))
+	noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;
     while (state->pc < end) {
 	code = *state->pc++;
 	if (wc_code(code) != WC_IF ||
@@ -570,9 +571,9 @@ execif(Estate state, int do_exec)
 	if (olderrexit)
 	    noerrexit = olderrexit;
 	else if (lastval)
-	    noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN | NOERREXIT_UNTIL_EXEC;
+	    noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN | NOERREXIT_UNTIL_EXEC;
 	else
-	    noerrexit = 0;
+	    noerrexit &= ~ (NOERREXIT_EXIT | NOERREXIT_RETURN);
 	cmdpush(run == 2 ? CS_ELSE : (s ? CS_ELIFTHEN : CS_IFTHEN));
 	execlist(state, 1, do_exec);
 	cmdpop();
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index f8a1231..5794459 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -661,6 +661,25 @@ F:Must be tested with a top-level script rather than source or function
 >before-out
 >before-in
 
+  mkdir -p zdotdir
+  print >zdotdir/.zshenv '
+  setopt norcs errreturn
+  fn() {
+    if false; then
+      print Bad
+    else
+      print Good
+    fi
+    print Better
+  }
+  fn
+  print In .zshenv'
+  ZDOTDIR=$PWD/zdotdir $ZTST_testdir/../Src/zsh -c 'true'
+0:ERR_RETURN within initialisation code with special flags
+>Good
+>Better
+>In .zshenv
+
   (setopt err_exit
   for x in y; do
     false && true

Re: err_return in initialization scripts returns from conditionals

[Peter Stephenson]

On Thu, 31 Aug 2017 10:35:47 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
 On Wed, 30 Aug 2017 20:36:48 +0000
> Jan Alexander Steffens <jan.steffens@gmail.com> wrote:
> > Since recently (5.4.2?), there seems to be a bug involving err_return
> > during initialization.
> > 
> > If I have a .zshrc with the following content:
> > 
> >     setopt err_return
> >     true && function {
> >       if false; then :; fi
> >       echo 1
> >     }
> >     echo 2
> > 
> > Then the "if false" causes the function to return and "1" is not printed.
> > Note that the behavior differs from running this file using "zsh .zshrc",
> > where both lines are printed, which I expect is the intended behavior.
> 
> Yes, err_return (and err_exit which isn't affected by this problem) are
> supposed to be suppressed at that point in initialisation as in normal
> code.  This shows up because initialisation is already specially handled
> and this was complicating the check for the state at that point.

I came up with another case (also added as a test) that made me realise
I was missing the point --- with the bit flags, we don't actually need
the "if (!noerrxit)" at all, we can simply "or" in the bits we need here
unconditionally, and fluffy bunnies leap and gambol in the fields (have
not confirmed this).

pws

diff --git a/Src/loop.c b/Src/loop.c
index 4859c97..40e3bcb 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -541,8 +541,7 @@ execif(Estate state, int do_exec)
     olderrexit = noerrexit;
     end = state->pc + WC_IF_SKIP(code);
 
-    if (!noerrexit)
-	noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
+    noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;
     while (state->pc < end) {
 	code = *state->pc++;
 	if (wc_code(code) != WC_IF ||
@@ -570,9 +569,9 @@ execif(Estate state, int do_exec)
 	if (olderrexit)
 	    noerrexit = olderrexit;
 	else if (lastval)
-	    noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN | NOERREXIT_UNTIL_EXEC;
+	    noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN | NOERREXIT_UNTIL_EXEC;
 	else
-	    noerrexit = 0;
+	    noerrexit &= ~ (NOERREXIT_EXIT | NOERREXIT_RETURN);
 	cmdpush(run == 2 ? CS_ELSE : (s ? CS_ELIFTHEN : CS_IFTHEN));
 	execlist(state, 1, do_exec);
 	cmdpop();
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index f8a1231..f229625 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -661,6 +661,40 @@ F:Must be tested with a top-level script rather than source or function
 >before-out
 >before-in
 
+  mkdir -p zdotdir
+  print >zdotdir/.zshenv '
+  setopt norcs errreturn
+  fn() {
+    if false; then
+      print Bad
+    else
+      print Good
+    fi
+    print Better
+  }
+  fn
+  print In .zshenv'
+  ZDOTDIR=$PWD/zdotdir $ZTST_testdir/../Src/zsh -c 'true'
+0:ERR_RETURN within initialisation code with special flags
+>Good
+>Better
+>In .zshenv
+
+  fn2() {
+    if false; then
+      print Bad
+    else
+      print Good
+    fi
+  }
+  fn() {
+    setopt err_return
+    fn2 || true
+  }
+  fn
+0:ERR_RETURN in "else" branch in nested function
+>Good
+
   (setopt err_exit
   for x in y; do
     false && true