Re: BUG: $_ empty on null function call

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Mon, 9 Feb 2015 14:25:07 +0000
Peter Stephenson <p.stephenson@xxxxxxxxxxx> wrote:
> On Mon, 9 Feb 2015 14:10:26 +0000
> Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > There are some other differences between anonymous functions, e.g., they
> > don't honor PRINT_EXIT_VALUE:
> >
> > Engineering-wise, the ideal solution would be for anonymous and named
> > functions to share code... though I realize that may be a somewhat
> > invasive code change.
> 
> They already do everywhere that doesn't deal with the special argument
> syntax (Micah's problem) or with immediate execution after a definition.
> I suspect this may have to do with a different path owing to an
> optimisation later in the execution path where we make certain
> assumptions if code is regarded as "simple".

Sigh.  It's a combination of that *and* execution immediately after
definition.

When the code is parsed, we don't know if PRINTEXITVALUE is going to be
set when it's run.  At this point I think we declare "simple" code
execution for anonymous functions dead in the water.  The effect is
probably small anyway.

It looks like we can make some code in the lowest level of general
command execution, execcmd(), run in a few more cases, at least the
following attempt to move them out of an if block doesn't cause any test
failures.

This doesn't help with Micah's problem which is due to the *third*
difference.

I suppose saying things like 'does anybody know why PRINTEXITVALUE only
works if the shell is executing code from standard input' is a bit
pointless?  It's documented as "only at the command line in interactive
shells", but that's not actually how it's implemented.

diff --git a/Src/exec.c b/Src/exec.c
index 3b0e936..992bd08 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2427,6 +2427,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
     wordcode code;
     Wordcode beg = state->pc, varspc;
     FILE *oxtrerr = xtrerr, *newxtrerr = NULL;
+    LinkList restorelist = 0, removelist = 0;
 
     doneps4 = 0;
     redir = (wc_code(*state->pc) == WC_REDIR ? ecgetredirs(state) : NULL);
@@ -3374,7 +3375,6 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    } else
 		lastval = (execfuncs[type - WC_CURSH])(state, do_exec);
 	} else if (is_builtin || is_shfunc) {
-	    LinkList restorelist = 0, removelist = 0;
 	    /* builtin or shell function */
 
 	    if (!forked && ((cflags & BINF_COMMAND) ||
@@ -3424,29 +3424,6 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		} else
 		    clearerr(stdout);
 	    }
-	    if (isset(PRINTEXITVALUE) && isset(SHINSTDIN) &&
-		lastval && !subsh) {
-#if defined(ZLONG_IS_LONG_LONG) && defined(PRINTF_HAS_LLD)
-		fprintf(stderr, "zsh: exit %lld\n", lastval);
-#else
-		fprintf(stderr, "zsh: exit %ld\n", (long)lastval);
-#endif
-		fflush(stderr);
-	    }
-
-	    if (do_exec) {
-		if (subsh)
-		    _exit(lastval);
-
-		/* If we are exec'ing a command, and we are not in a subshell, *
-		 * then check if we should save the history file.              */
-		if (isset(RCS) && interact && !nohistsave)
-		    savehistfile(NULL, 1, HFILE_USE_OPTIONS);
-		exit(lastval);
-	    }
-	    if (restorelist)
-		restore_params(restorelist, removelist);
-
 	} else {
 	    if (!forked)
 		setiparam("SHLVL", --shlvl);
@@ -3496,6 +3473,28 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		execlist(state, 0, 1);
 	    }
 	}
+	if (isset(PRINTEXITVALUE) && isset(SHINSTDIN) &&
+	    lastval && !subsh) {
+#if defined(ZLONG_IS_LONG_LONG) && defined(PRINTF_HAS_LLD)
+	    fprintf(stderr, "zsh: exit %lld\n", lastval);
+#else
+	    fprintf(stderr, "zsh: exit %ld\n", (long)lastval);
+#endif
+	    fflush(stderr);
+	}
+
+	if (do_exec) {
+	    if (subsh)
+		_exit(lastval);
+
+	    /* If we are exec'ing a command, and we are not in a subshell, *
+	     * then check if we should save the history file.              */
+	    if (isset(RCS) && interact && !nohistsave)
+		savehistfile(NULL, 1, HFILE_USE_OPTIONS);
+	    exit(lastval);
+	}
+	if (restorelist)
+	    restore_params(restorelist, removelist);
     }
 
   err:
diff --git a/Src/parse.c b/Src/parse.c
index 0b54a90..ffd25de 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1612,8 +1612,7 @@ par_funcdef(int *cmplx)
 	    num++;
 	    zshlex();
 	}
-	if (num > 0)
-	    *cmplx = 1;
+	*cmplx = 1;
 	ecbuf[parg] = ecused - parg; /*?*/
 	ecbuf[parg+1] = num;
     }
@@ -1897,8 +1896,7 @@ par_simple(int *cmplx, int nr)
 		    argc++;
 		    zshlex();
 		}
-		if (argc > 0)
-		    *cmplx = 1;
+		*cmplx = 1;
 		ecbuf[parg] = ecused - parg; /*?*/
 		ecbuf[parg+1] = argc;
 	    }
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 46b1837..3213534 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -783,14 +783,24 @@
 >print is a shell builtin
 ?(eval):8: command not found: print
 
-# This option seems to be problematic.  I don't quite know how it works.
-##   func() {
-##     setopt localoptions printexitvalue
-##     false
-##   }
-##   func
-## 1:PRINT_EXIT_VALUE option
-## ?(eval):2: exit 1
+# PRINTEXITVALUE only works if shell input is coming from standard input.
+# Goodness only knows why.
+  $ZTST_testdir/../Src/zsh -f <<<'
+      setopt printexitvalue
+      func() {
+	  false
+      }
+      func
+  '
+1:PRINT_EXIT_VALUE option
+?zsh: exit 1
+
+  $ZTST_testdir/../Src/zsh -f <<<'
+      setopt printexitvalue
+      () { false; }
+  '
+1:PRINT_EXIT_VALUE option for anonymous function
+?zsh: exit 1
 
   setopt promptbang
   print -P !