Re: question about posixbuiltins

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Mon, 13 Apr 2015 14:44:52 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Apr 12, 2015 4:03 PM, "Han Pingtian" <hanpt@xxxxxxxxxxxxxxxxxx> wrote:
> > POSIX_BUILTINS <K> <S>
> >        [...]  Parameter  assignments  specified
> >        before  shell  functions and special builtins are kept after the
> >        command completes unless the special builtin  is  prefixed  with
> >        the  command  builtin.
> > [...]
> > But looks like the parameter assignment before "command special-builtin"
> > is kept even though POSIX_BUILTINS has been set
> 
> Indeed I can confirm, it appears this either was never working or has been
> broken by some recent change.

(Moved to zsh-workers.)

I can believe it was never working --- the logic is horrifically
tortuous.  I've rewritten it to try to make sense of it.  The compiler
will be grateful for a variable to optimise out and perhaps therefore
indulgent enough to ignore remaining mistakes.

I don't really understand the non-POSIX_BUILTINS case, but unless my
brain has totally failed to cope it's the same logic it's always been.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 1a6149a..2ee37d0 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3384,14 +3384,28 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    LinkList restorelist = 0, removelist = 0;
 	    /* builtin or shell function */
 
-	    if (!forked && ((cflags & BINF_COMMAND) ||
-			    (unset(POSIXBUILTINS) && !assign) ||
-			    (isset(POSIXBUILTINS) && !is_shfunc &&
-			     !(hn->flags & BINF_PSPECIAL)))) {
-		if (varspc)
+	    if (!forked && varspc) {
+		int do_save = 0;
+		if (isset(POSIXBUILTINS)) {
+		    /*
+		     * If it's a function or special builtin --- save
+		     * if it's got "command" in front.
+		     * If it's a normal command --- save.
+		     */
+		    if (is_shfunc || (hn->flags & BINF_PSPECIAL))
+			do_save = (orig_cflags & BINF_COMMAND);
+		    else
+			do_save = 1;
+		} else {
+		    /*
+		     * Save if it's got "command" in front or it's
+		     * not a magic-equals assignment.
+		     */
+		    if ((cflags & BINF_COMMAND) || !assign)
+			do_save = 1;
+		}
+		if (do_save)
 		    save_params(state, varspc, &restorelist, &removelist);
-		else
-		    restorelist = removelist = NULL;
 	    }
 	    if (varspc) {
 		/* Export this if the command is a shell function,
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 3213534..5c453c8 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -783,6 +783,19 @@
 >print is a shell builtin
 ?(eval):8: command not found: print
 
+  # With non-special command: original value restored
+  # With special builtin: new value kept
+  # With special builtin preceeded by "command": original value restored.
+  (setopt posixbuiltins
+  FOO=val0
+  FOO=val1 true; echo $FOO
+  FOO=val2 times 1>/dev/null 2>&1; echo $FOO
+  FOO=val3 command times 1>/dev/null 2>&1; echo $FOO)
+0:POSIX_BUILTINS and restoring variables
+>val0
+>val2
+>val2
+
 # PRINTEXITVALUE only works if shell input is coming from standard input.
 # Goodness only knows why.
   $ZTST_testdir/../Src/zsh -f <<<'