BUG: $_ empty on null function call

BUG: $_ empty on null function call

[Micah Waddoups]

[-- Attachment #1.1: Type: text/plain, Size: 1648 bytes --]

Hello all,
   I am still a beginner in developing, or I would tackle this issue 
directly myself.  The misbehavior is when the command list has a null 
function with parameters, the last-command-last-parameter ($_) built-in 
variable goes blank.  Example:

|local a=1 b=2 c=3; : One; function { : Two; echo $_; print -l $argv; } 
$_ Three; print -l $_ Four;
## Should print:
#  Two
#  One
#  Three
#  Three
#  Four

## Currently prints:
#  Two
#  Three
#  Four

## Conversely, when no arguments are given to the null function,
## it operates mostly normally, but as if the function were not there:|
||local a=1 b=2 c=3; : One; function { : Two; echo $_; print -l $argv; 
}; print -l $_ Four;|
## Outputs:
#  Two
#
#  One
#  Four
##     ( second line is 'print -l' but $argv is empty
|

   It would seem this unexpected behavior may be the result of an 
unfinished shell source where the best shell behavior was not clear to 
the programmer.  I admit dealing with null functions is a minor point of 
dilema with script writing, but I have gotten super intricate with the 
beautiful script design and features made available to me by ZSH, and I 
have found that the behavior suggested at "Should print" above is the 
only way that is consistent in a large script where an alias may use a 
null function for better handling of parameters without interfering with 
any currently same-name functions, or when a Heredoc is used and one 
wants to grab the last argument to use as the parameter of an 
immediately following null function.

-- 
Micah micah@askmicah.net <mailto:micah@askmicah.net>
AskMicah.Net <http://askmicah.net>, Problem Solving Agency

[-- Attachment #1.2: Type: text/html, Size: 2675 bytes --]

Re: BUG: $_ empty on null function call

[Micah Waddoups]

[-- Attachment #1.1: Type: text/plain, Size: 2098 bytes --]

Again, please show mercy on me in my innexperience.  The current 
patch-level of the ZSH shell I am using is:
    zsh-5.0.7-0-g208bded
It is the standard "Arch Linux" zsh package as of January 2015.


On 02/09/2015 03:38 AM, Micah Waddoups wrote:
> Hello all,
>   I am still a beginner in developing, or I would tackle this issue 
> directly myself.  The misbehavior is when the command list has a null 
> function with parameters, the last-command-last-parameter ($_) 
> built-in variable goes blank. Example:
>
> |local a=1 b=2 c=3; : One; function { : Two; echo $_; print -l $argv; 
> } $_ Three; print -l $_ Four;
> ## Should print:
> #  Two
> #  One
> #  Three
> #  Three
> #  Four
>
> ## Currently prints:
> #  Two
> #  Three
> #  Four
>
> ## Conversely, when no arguments are given to the null function,
> ## it operates mostly normally, but as if the function were not there:|
> ||local a=1 b=2 c=3; : One; function { : Two; echo $_; print -l $argv; 
> }; print -l $_ Four;|
> ## Outputs:
> #  Two
> #
> #  One
> #  Four
> ##     ( second line is 'print -l' but $argv is empty
> |
>
>   It would seem this unexpected behavior may be the result of an 
> unfinished shell source where the best shell behavior was not clear to 
> the programmer.  I admit dealing with null functions is a minor point 
> of dilema with script writing, but I have gotten super intricate with 
> the beautiful script design and features made available to me by ZSH, 
> and I have found that the behavior suggested at "Should print" above 
> is the only way that is consistent in a large script where an alias 
> may use a null function for better handling of parameters without 
> interfering with any currently same-name functions, or when a Heredoc 
> is used and one wants to grab the last argument to use as the 
> parameter of an immediately following null function.
>
> -- 
> Micah micah@askmicah.net <mailto:micah@askmicah.net>
> AskMicah.Net <http://askmicah.net>, Problem Solving Agency

-- 
Micah micah@askmicah.net <mailto:micah@askmicah.net>
AskMicah.Net <http://askmicah.net>, Problem Solving Agency

[-- Attachment #1.2: Type: text/html, Size: 3480 bytes --]

Re: BUG: $_ empty on null function call

[Peter Stephenson]

On Mon, 9 Feb 2015 03:38:58 -0800
Micah Waddoups <micah@askmicah.net> wrote:
>    I am still a beginner in developing, or I would tackle this issue 
> directly myself.  The misbehavior is when the command list has a null 
> function with parameters, the last-command-last-parameter ($_) built-in 
> variable goes blank.

Yes, that's obviously an oversight internally.  $_ simply isn't handled
on this path.

Somebody will get around to it eventually, I imagine...

pws

Re: BUG: $_ empty on null function call

[Daniel Shahaf]

Peter Stephenson wrote on Mon, Feb 09, 2015 at 12:20:42 +0000:
> On Mon, 9 Feb 2015 03:38:58 -0800
> Micah Waddoups <micah@askmicah.net> wrote:
> >    I am still a beginner in developing, or I would tackle this issue 
> > directly myself.  The misbehavior is when the command list has a null 
> > function with parameters, the last-command-last-parameter ($_) built-in 
> > variable goes blank.
> 
> Yes, that's obviously an oversight internally.  $_ simply isn't handled
> on this path.
> 

There are some other differences between anonymous functions, e.g., they
don't honor PRINT_EXIT_VALUE:

    % setopt printexitvalue
    % () false

v.

    % setopt printexitvalue
    % f() false
    % f                                                            
    zsh: exit 1

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.

Daniel

> Somebody will get around to it eventually, I imagine...
> 
> pws

Re: BUG: $_ empty on null function call

[Peter Stephenson]

On Mon, 9 Feb 2015 14:10:26 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> 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".  This is inevitably a
maintenance headache and we've had lots of issues there.

By the way, if that means you're interested in looking at the internals,
please do...  suggestions based on this are much more likely to be
useful, for obvious reasons.

pws

Re: BUG: $_ empty on null function call

[Peter Stephenson]

On Mon, 9 Feb 2015 14:25:07 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Mon, 9 Feb 2015 14:10:26 +0000
> Daniel Shahaf <d.s@daniel.shahaf.name> 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 !

Re: BUG: $_ empty on null function call

[Bart Schaefer]

On Feb 9,  3:13pm, Peter Stephenson wrote:
}
} 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?

There seem to be (perhaps "have been" would be more accurate as I think
we've changed some of them) several places where SHINSTDIN was used as
a placeholder for "stderr is a terminal, therefore we can print this
without garbling output that may be going where the user won't see it".

Re: BUG: $_ empty on null function call

[Daniel Shahaf]

Peter Stephenson wrote on Mon, Feb 09, 2015 at 14:25:07 +0000:
> On Mon, 9 Feb 2015 14:10:26 +0000
> Daniel Shahaf <d.s@daniel.shahaf.name> 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.

Sorry, I know they have execshfunc() in common.  I was trying to suggest
that anonymous functions should start using execcmd(), which named
functions were using, in order to reuse its handling of $_ and
PRINT_EXIT_VALUE.

> 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".  This is inevitably a
> maintenance headache and we've had lots of issues there.
> 

For what it's worth, "() {} foo bar ; print $_" reproduces Micah's
problem and, both before and after your patch, doesn't go through
execsimple().

Cheers,

Daniel

Re: BUG: $_ empty on null function call

[Daniel Shahaf]

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

Peter Stephenson wrote on Mon, Feb 09, 2015 at 15:13:40 +0000:
> On Mon, 9 Feb 2015 14:25:07 +0000
> Peter Stephenson <p.stephenson@samsung.com> wrote:
> > On Mon, 9 Feb 2015 14:10:26 +0000
> > Daniel Shahaf <d.s@daniel.shahaf.name> 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.
> 

I was looking at making [[ honor PRINT_EXIT_VALUE; right now it doesn't,
because it uses execlist->execsimple->execcond (and so never passes
through execcmd).  I'm mentioning that since it may be relevant, as it
also concerns a simple command wanting to honor PRINT_EXIT_VALUE.

¹ The use-case: I use [[ ]] as a standalone command (not as part of an
if or while) in interactive shells to test its syntax when writing scripts.

> 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'll just point out for anyone who needs $_ with anonymous functions
working "yesterday" that a quick and dirty way to achieve that is via
the attached patch.  It might not be a good general solution since it
duplicates code, but it does make invocation of anonymous functions set
$_ and I think it has no harmful effects.

I suppose a better fix would involve extracting the arguments of an
anonymous function up in execcmd() rather than down in execfuncdef(), so
they can reuse the existing setunderscore() call in execcmd()?

> +      () { false; }
> +1:PRINT_EXIT_VALUE option for anonymous function
> +?zsh: exit 1

Thanks for fixing this :)

Daniel

[-- Attachment #2: anonfunc-dollarunderscore.diff --]
[-- Type: text/x-patch, Size: 434 bytes --]

diff --git a/Src/exec.c b/Src/exec.c
index 3b0e936..719345e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4482,6 +4482,11 @@ execfuncdef(Estate state, Eprog redir_prog)
 	    shf->node.nam = "(anon)";
 	    pushnode(args, shf->node.nam);
 
+	    /* Set up special parameter $_ */
+	    setunderscore((args && nonempty(args))
+			  ? ((char *) getdata(lastnode(args)))
+			  : "");
+
 	    execshfunc(shf, args);
 	    ret = lastval;