[PATCH] 'functions -T' tracing: recurse into anonymous functions.

[PATCH] 'functions -T' tracing: recurse into anonymous functions.

[Daniel Shahaf]

---
 Doc/Zsh/builtins.yo |  2 +-
 Src/exec.c          | 12 +++++++++---
 Test/E02xtrace.ztst | 17 +++++++++++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 1ca1f24..813b725 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -2029,7 +2029,7 @@ can be made, and the only other valid flags are tt(-t), tt(-T), tt(-k),
 tt(-u), tt(-U) and tt(-z).  The flag tt(-t) turns on execution tracing
 for this function; the flag tt(-T) does the same, but turns off tracing
 on any function called from the present one, unless that function also
-has the tt(-t) or tt(-T) flag.  The tt(-u) and tt(-U) flags cause the
+has the tt(-t) or tt(-T) flag or is anonymous.  The tt(-u) and tt(-U) flags cause the
 function to be marked for autoloading; tt(-U) also causes alias
 expansion to be suppressed when the function is loaded.  See the
 description of the `tt(autoload)' builtin for details.
diff --git a/Src/exec.c b/Src/exec.c
index 2dcd5bc..515406f 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4615,6 +4615,8 @@ exectime(Estate state, UNUSED(int do_exec))
 
 /* Define a shell function */
 
+static const char *const ANONYMOUS_FUNCTION_NAME = "(anon)";
+
 /**/
 static int
 execfuncdef(Estate state, Eprog redir_prog)
@@ -4732,7 +4734,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 
 	    if (!args)
 		args = newlinklist();
-	    shf->node.nam = "(anon)";
+	    shf->node.nam = (char *) ANONYMOUS_FUNCTION_NAME;
 	    pushnode(args, shf->node.nam);
 
 	    execshfunc(shf, args);
@@ -5165,8 +5167,12 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 
 	if (flags & (PM_TAGGED|PM_TAGGED_LOCAL))
 	    opts[XTRACE] = 1;
-	else if (oflags & PM_TAGGED_LOCAL)
-	    opts[XTRACE] = 0;
+	else if (oflags & PM_TAGGED_LOCAL) {
+	    if (shfunc->node.nam == ANONYMOUS_FUNCTION_NAME /* pointer comparison */)
+		flags |= PM_TAGGED_LOCAL;
+	    else
+		opts[XTRACE] = 0;
+	}
 	ooflags = oflags;
 	/*
 	 * oflags is static, because we compare it on the next recursive
diff --git a/Test/E02xtrace.ztst b/Test/E02xtrace.ztst
index 093a587..6e425e7 100644
--- a/Test/E02xtrace.ztst
+++ b/Test/E02xtrace.ztst
@@ -127,3 +127,20 @@
 ?+(eval):2> [[ 'f o' == f\ x* || 'b r' != z\ o && 'squashy sound' < 'squishy sound' ]]
 ?+(eval):3> [[ -e nonexistentfile || -z '' && -t 3 ]]
 ?+(eval):4> set +x
+
+  # Part 1: Recurses into nested anonymous functions
+  fn() {
+    () { () { true } }
+  }
+  functions -T fn
+  fn
+  # Part 2: Doesn't recurse into named functions
+  gn() { true }
+  fn() { gn }
+  functions -T fn
+  fn
+0:tracing recurses into anonymous functions
+?+fn:1> '(anon)'
+?+(anon):0> '(anon)'
+?+(anon):0> true
+?+fn:0> gn

Re: [PATCH] 'functions -T' tracing: recurse into anonymous functions.

[Bart Schaefer]

On Jun 10,  5:37pm, Daniel Shahaf wrote:
}
}  for this function; the flag tt(-T) does the same, but turns off tracing
}  on any function called from the present one, unless that function also
} -has the tt(-t) or tt(-T) flag.  The tt(-u) and tt(-U) flags cause the
} +has the tt(-t) or tt(-T) flag or is anonymous.  The tt(-u) and tt(-U) flags cause the

That phrasing is a little hard to follow.  Perhaps something more like:

... turns off tracing
for any named (not anonymous) function called from the present one, unless
that function also ...

Note I'm also suggesting changing "turns off tracing on" into "turns off
tracing for" to reduce the chances of off/on confusion.  (Further, don't
be afraid to reformat paragraphs if lines get long.)

I've been considering something similar for the zsh/param/private module
to make caller private variables visible inside anonymous functions, but
haven't worked out all the necessary stack management.

Re: [PATCH] 'functions -T' tracing: recurse into anonymous functions.

[Daniel Shahaf]

Bart Schaefer wrote on Fri, Jun 10, 2016 at 21:06:32 -0700:
> On Jun 10,  5:37pm, Daniel Shahaf wrote:
> }
> }  for this function; the flag tt(-T) does the same, but turns off tracing
> }  on any function called from the present one, unless that function also
> } -has the tt(-t) or tt(-T) flag.  The tt(-u) and tt(-U) flags cause the
> } +has the tt(-t) or tt(-T) flag or is anonymous.  The tt(-u) and tt(-U) flags cause the
> 
> That phrasing is a little hard to follow.  Perhaps something more like:
> 
> ... turns off tracing
> for any named (not anonymous) function called from the present one, unless
> that function also ...
> 
> Note I'm also suggesting changing "turns off tracing on" into "turns off
> tracing for" to reduce the chances of off/on confusion.

Good call.  Will fix before pushing.

> (Further, don't be afraid to reformat paragraphs if lines get long.)
> 

I generally avoid rewrapping paragraphs completely since default diff
tools tend to be line-based and therefore don't render such diffs
readably.

I'll break the long line into two.  (That will leave a short line, similar
to 38155.  I can rewrap the paragraph if a short line is a problem.)

> I've been considering something similar for the zsh/param/private module
> to make caller private variables visible inside anonymous functions, but
> haven't worked out all the necessary stack management.

I've also been considering something similar: I'd like PRINT_EXIT_VALUE
to recurse into anonymous functions.  IIRC the problem I ran into was
that the return value of the last command in the anon func, and of the
anon func itself, were both printed (despite being equal).