zsh-workers
 help / color / mirror / code / Atom feed
* RFC: function -T f { … }
@ 2020-03-18 18:30 Daniel Shahaf
  2020-03-18 19:53 ` Peter Stephenson
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Shahaf @ 2020-03-18 18:30 UTC (permalink / raw)
  To: zsh-workers

How about adding a «function -T f { … }» syntax, that defines a
function and enables tracing for it simultaneously?

I've often found myself going to the first line of a function, copying
its name, then going to the closing brace of the function and adding
a «functions -T $thefunctionsname» statement there, in order to have
tracing enabled during multiple «zsh -f» runs.

It's backwards incompatible, but not more than «printf -v» was.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: RFC: function -T f { … }
  2020-03-18 18:30 RFC: function -T f { … } Daniel Shahaf
@ 2020-03-18 19:53 ` Peter Stephenson
  2020-03-20  2:04   ` Daniel Shahaf
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Stephenson @ 2020-03-18 19:53 UTC (permalink / raw)
  To: zsh-workers

On Wed, 2020-03-18 at 18:30 +0000, Daniel Shahaf wrote:
> How about adding a «function -T f { … }» syntax, that defines a
> function and enables tracing for it simultaneously?
> 
> I've often found myself going to the first line of a function, copying
> its name, then going to the closing brace of the function and adding
> a «functions -T $thefunctionsname» statement there, in order to have
> tracing enabled during multiple «zsh -f» runs.
> 
> It's backwards incompatible, but not more than «printf -v» was.

Can't see any objection, and can't see an easy workaround.

It does work for autoloads --- you can turn on xtrace before the
function is loaded.

pws


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: RFC: function -T f { … }
  2020-03-18 19:53 ` Peter Stephenson
@ 2020-03-20  2:04   ` Daniel Shahaf
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Shahaf @ 2020-03-20  2:04 UTC (permalink / raw)
  To: zsh-workers

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

Peter Stephenson wrote on Wed, 18 Mar 2020 19:53 +0000:
> On Wed, 2020-03-18 at 18:30 +0000, Daniel Shahaf wrote:
> > How about adding a «function -T f { … }» syntax, that defines a
> > function and enables tracing for it simultaneously?
> > 
> > I've often found myself going to the first line of a function, copying
> > its name, then going to the closing brace of the function and adding
> > a «functions -T $thefunctionsname» statement there, in order to have
> > tracing enabled during multiple «zsh -f» runs.
> > 
> > It's backwards incompatible, but not more than «printf -v» was.  
> 
> Can't see any objection, and can't see an easy workaround.

Thanks for the sanity check, Peter.

Patch series attached.

The first six patches are no-ops; the functional change is confined to
the last three patches.  Furthermore, patch #9 changes some of the lines
added by patch #8, so y'all may find it easier to review the cumulative
diff of those two patches¹ than to review them one by one.

Regarding the man page patch, I wanted to have have the documentation
of «function» hyperlink directly to the documentation of «typeset» (in
the HTML/PDF output formats), but didn't figure out how to do this.

> It does work for autoloads --- you can turn on xtrace before the
> function is loaded.

*nod* «autoload -T foo» and «autoload foo; functions -T foo» both work;
however, in my use-cases I'm normally trying to trace a function that's
defined in the middle of a *.zsh file that also defines multiple other
functions.

Cheers,

Daniel

¹ For example, using «combinediff <8-9>*» or «git diff HEAD^^».

[-- Attachment #2: 0001-internal-Remove-a-redundant-assignment.patch.txt --]
[-- Type: text/plain, Size: 737 bytes --]

From 4a8d83f9b86f0ad6041640e9ad28781ad440036e Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 18 Mar 2020 19:42:08 +0000
Subject: [PATCH 1/9] internal: Remove a redundant assignment.

The value is overwritten five lines below, without being read in the interim.
---
 Src/exec.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Src/exec.c b/Src/exec.c
index bca051d4f..cd014ff38 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5166,7 +5166,6 @@ execfuncdef(Estate state, Eprog redir_prog)
 
     end = beg + WC_FUNCDEF_SKIP(state->pc[-1]);
     names = ecgetlist(state, *state->pc++, EC_DUPTOK, &htok);
-    nprg = end - beg;
     sbeg = *state->pc++;
     nstrs = *state->pc++;
     npats = *state->pc++;

[-- Attachment #3: 0002-internal-Reduce-some-variables-visibility.-No-fu.patch.txt --]
[-- Type: text/plain, Size: 881 bytes --]

From e4ad3e10812fd54a46a91f08b7943ea18b813a55 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 18 Mar 2020 17:45:23 +0000
Subject: [PATCH 2/9] internal: Reduce some variables' visibility. No
 functional change.

---
 Src/parse.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/Src/parse.c b/Src/parse.c
index de1b27967..bd974a573 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -253,14 +253,13 @@ struct heredocs *hdocs;
  * to avoid a lot of string parsing and some more string duplication.
  */
 
-/**/
-int eclen, ecused, ecnpats;
-/**/
-Wordcode ecbuf;
-/**/
-Eccstr ecstrs;
-/**/
-int ecsoffs, ecssub, ecnfunc;
+static int eclen, ecused, ecnpats;
+
+static Wordcode ecbuf;
+
+static Eccstr ecstrs;
+
+static int ecsoffs, ecssub, ecnfunc;
 
 #define EC_INIT_SIZE         256
 #define EC_DOUBLE_THRESHOLD  32768

[-- Attachment #4: 0003-internal-Add-some-comments-around-Eccstr.-No-fun.patch.txt --]
[-- Type: text/plain, Size: 3248 bytes --]

From cce0d2cfe2d4728d4dcbc2a56cb8243ecd58a80e Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Thu, 19 Mar 2020 21:14:42 +0000
Subject: [PATCH 3/9] internal: Add some comments around Eccstr. No functional
 change.

---
 Src/parse.c | 31 +++++++++++++++++++++++++++----
 Src/zsh.h   | 24 ++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/Src/parse.c b/Src/parse.c
index bd974a573..39d2172d4 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -253,13 +253,24 @@ struct heredocs *hdocs;
  * to avoid a lot of string parsing and some more string duplication.
  */
 
-static int eclen, ecused, ecnpats;
+/* Number of wordcodes allocated. */
+static int eclen;
+/* Number of wordcodes populated. */
+static int ecused;
+/* Number of patterns... */
+static int ecnpats;
 
 static Wordcode ecbuf;
 
 static Eccstr ecstrs;
 
-static int ecsoffs, ecssub, ecnfunc;
+static int ecsoffs, ecssub;
+
+/*
+ * ### The number of starts and ends of function definitions up to this point.
+ * Never decremented.
+ */
+static int ecnfunc;
 
 #define EC_INIT_SIZE         256
 #define EC_DOUBLE_THRESHOLD  32768
@@ -363,7 +374,11 @@ ecispace(int p, int n)
     ecadjusthere(p, n);
 }
 
-/* Add one wordcode. */
+/* 
+ * Add one wordcode.
+ *
+ * Return the index of the added wordcode.
+ */
 
 static int
 ecadd(wordcode c)
@@ -402,6 +417,7 @@ ecstrcode(char *s)
     unsigned val = hasher(s);
 
     if ((l = strlen(s) + 1) && l <= 4) {
+	/* Short string. */
 	t = has_token(s);
 	wordcode c = (t ? 3 : 2);
 	switch (l) {
@@ -412,11 +428,13 @@ ecstrcode(char *s)
 	}
 	return c;
     } else {
+	/* Long string. */
 	Eccstr p, *pp;
 	long cmp;
 
 	for (pp = &ecstrs; (p = *pp); ) {
 	    if (!(cmp = p->nfunc - ecnfunc) && !(cmp = (((long)p->hashval) - ((long)val))) && !(cmp = strcmp(p->str, s))) {
+		/* Re-use the existing string. */
 		return p->offs;
             }
 	    pp = (cmp < 0 ? &(p->left) : &(p->right));
@@ -493,7 +511,12 @@ init_parse(void)
 
 /* Build eprog. */
 
-/* careful: copy_ecstr is from arg1 to arg2, unlike memcpy */
+/*
+ * Copy the strings of s and all its descendants in the binary tree to the
+ * memory block p.
+ *
+ * careful: copy_ecstr is from arg1 to arg2, unlike memcpy
+ */
 
 static void
 copy_ecstr(Eccstr s, char *p)
diff --git a/Src/zsh.h b/Src/zsh.h
index 834142895..82d152bb8 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -832,13 +832,33 @@ struct estate {
     char *strs;			/* strings from prog */
 };
 
+/* 
+ * A binary tree of strings.
+ *
+ * Refer to the "Word code." comment at the top of Src/parse.c for details.
+ */
 typedef struct eccstr *Eccstr;
-
 struct eccstr {
+    /* Child pointers. */
     Eccstr left, right;
+
+    /* String; pointer into to estate::strs. */
     char *str;
-    wordcode offs, aoffs;
+
+    /* Wordcode of a long string, as described in the Src/parse.c comment. */
+    wordcode offs;
+
+    /* Raw memory offset of str in estate::strs. */
+    wordcode aoffs;
+
+    /* 
+     * ### The number of starts and ends of function definitions up to this point.
+     *
+     * String reuse may only happen between strings that have the same "nfunc" value.
+     */
     int nfunc;
+
+    /* Hash of str. */
     int hashval;
 };
 

[-- Attachment #5: 0004-internal-Add-some-comments-around-wordcodes.-No-.patch.txt --]
[-- Type: text/plain, Size: 1717 bytes --]

From e217f6e22e5c88ed31b1cafac0c75efa372c3ba6 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Thu, 19 Mar 2020 21:15:54 +0000
Subject: [PATCH 4/9] internal: Add some comments around wordcodes. No
 functional change.

---
 Src/parse.c | 7 +++++++
 Src/zsh.h   | 8 ++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Src/parse.c b/Src/parse.c
index 39d2172d4..666595ef1 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -102,6 +102,13 @@ struct heredocs *hdocs;
  * The parser now produces word code, reducing memory consumption compared
  * to the nested structs we had before.
  *
+ * Word codes are represented by the "wordcode" type.
+ *
+ * Each wordcode variable consists of a "code", in the least-significant bits
+ * of the value, and "data" in the other bits.  The macros wc_code() and wc_data()
+ * access the "code" and "data" parts of a wordcode.  The macros wc_bdata() and
+ * wc_bld() build wordcodes from code and data.
+ *
  * Word code layout:
  *
  *   WC_END
diff --git a/Src/zsh.h b/Src/zsh.h
index 82d152bb8..d72c338da 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -866,8 +866,8 @@ struct eccstr {
 #define EC_DUP    1
 #define EC_DUPTOK 2
 
+/* See comment at the top of Src/parse.c for details. */
 #define WC_CODEBITS 5
-
 #define wc_code(C)   ((C) & ((wordcode) ((1 << WC_CODEBITS) - 1)))
 #define wc_data(C)   ((C) >> WC_CODEBITS)
 #define wc_bdata(D)  ((D) << WC_CODEBITS)
@@ -896,7 +896,11 @@ struct eccstr {
 #define WC_AUTOFN  20
 #define WC_TRY     21
 
-/* increment as necessary */
+/* 
+ * Increment as necessary.
+ * 
+ * If this exceeds 31, increment WC_CODEBITS.
+ */
 #define WC_COUNT   22
 
 #define WCB_END()           wc_bld(WC_END, 0)

[-- Attachment #6: 0005-internal-Document-the-WC_FUNCDEF-data-layout-for.patch.txt --]
[-- Type: text/plain, Size: 1523 bytes --]

From edbe98a63a704b0cb2db52f3ed6473f631e04e41 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Thu, 19 Mar 2020 21:16:12 +0000
Subject: [PATCH 5/9] internal: Document the WC_FUNCDEF data layout for
 anonymous functions with arguments (follow-up to 29492)

---
 Src/parse.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Src/parse.c b/Src/parse.c
index 666595ef1..acbf42a9c 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -175,6 +175,10 @@ struct heredocs *hdocs;
  *     - followed by number of patterns for body
  *     - followed by codes for body
  *     - followed by strings for body
+ *     - if number of names is 0, followed by:
+ *       - the offset to the end of the funcdef
+ *       - the number of arguments to the function
+ *       - the arguments to the function
  *
  *   WC_FOR
  *     - data contains type (list, ...) and offset to after body
@@ -1734,8 +1738,9 @@ par_funcdef(int *cmplx)
 
     ecbuf[p] = WCB_FUNCDEF(ecused - 1 - p);
 
+    /* If it's an anonymous function... */
     if (num == 0) {
-	/* Unnamed function */
+	/* ... look for arguments to it. */
 	int parg = ecadd(0);
 	ecadd(0);
 	while (tok == STRING) {
@@ -2110,8 +2115,9 @@ par_simple(int *cmplx, int nr)
 
 	    ecbuf[p] = WCB_FUNCDEF(ecused - 1 - p);
 
+	    /* If it's an anonymous function... */
 	    if (argc == 0) {
-		/* Unnamed function */
+		/* ... look for arguments to it. */
 		int parg = ecadd(0);
 		ecadd(0);
 		while (tok == STRING || IS_REDIROP(tok)) {

[-- Attachment #7: 0006-internal-Add-some-comments-for-orientation.-No-f.patch.txt --]
[-- Type: text/plain, Size: 1557 bytes --]

From c4b6bebf6e4e454c6a13c82c916c594cf300791f Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 18 Mar 2020 18:53:11 +0000
Subject: [PATCH 6/9] internal: Add some comments for orientation. No
 functional change.

---
 Src/parse.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Src/parse.c b/Src/parse.c
index acbf42a9c..8c14aa80b 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1676,7 +1676,7 @@ par_funcdef(int *cmplx)
     zshlex();
 
     p = ecadd(0);
-    ecadd(0);
+    ecadd(0); /* p + 1 */
 
     while (tok == STRING) {
 	if ((*tokstr == Inbrace || *tokstr == '{') &&
@@ -1688,9 +1688,9 @@ par_funcdef(int *cmplx)
 	num++;
 	zshlex();
     }
-    ecadd(0);
-    ecadd(0);
-    ecadd(0);
+    ecadd(0); /* p + num + 2 */
+    ecadd(0); /* p + num + 3 */
+    ecadd(0); /* p + num + 4 */
 
     nocorrect = 0;
     incmdpos = 1;
@@ -1728,15 +1728,15 @@ par_funcdef(int *cmplx)
 
     ecadd(WCB_END());
     ecbuf[p + num + 2] = so - oecssub;
-    ecbuf[p + num + 3] = ecsoffs - so;
-    ecbuf[p + num + 4] = ecnpats;
-    ecbuf[p + 1] = num;
+    ecbuf[p + num + 3] = ecsoffs - so; /* "length of string table" */
+    ecbuf[p + num + 4] = ecnpats; /* "number of patterns for body" */
+    ecbuf[p + 1] = num; /* "number of names" */
 
     ecnpats = onp;
     ecssub = oecssub;
     ecnfunc++;
 
-    ecbuf[p] = WCB_FUNCDEF(ecused - 1 - p);
+    ecbuf[p] = WCB_FUNCDEF(ecused - 1 - p); /* "offset to after body" */
 
     /* If it's an anonymous function... */
     if (num == 0) {

[-- Attachment #8: 0007-WC_FUNCDEF-Add-a-placeholder-element.patch.txt --]
[-- Type: text/plain, Size: 3069 bytes --]

From 745f602c16324a77a38a6d5290c7a11610380069 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 18 Mar 2020 19:57:49 +0000
Subject: [PATCH 7/9] WC_FUNCDEF: Add a placeholder element.

---
 Config/version.mk | 4 ++--
 Src/exec.c        | 3 ++-
 Src/parse.c       | 5 +++++
 Src/text.c        | 2 +-
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/Config/version.mk b/Config/version.mk
index 6540e4b98..7ecfd35ba 100644
--- a/Config/version.mk
+++ b/Config/version.mk
@@ -27,5 +27,5 @@
 # This must also serve as a shell script, so do not add spaces around the
 # `=' signs.
 
-VERSION=5.8.0.1-dev
-VERSION_DATE='February 15, 2020'
+VERSION=5.8.0.2-dev
+VERSION_DATE='March 19, 2020'
diff --git a/Src/exec.c b/Src/exec.c
index cd014ff38..3c3fcfa3e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5169,6 +5169,7 @@ execfuncdef(Estate state, Eprog redir_prog)
     sbeg = *state->pc++;
     nstrs = *state->pc++;
     npats = *state->pc++;
+    (void) *state->pc++;
 
     nprg = (end - state->pc);
     plen = nprg * sizeof(wordcode);
@@ -6138,7 +6139,7 @@ stripkshdef(Eprog prog, char *name)
 	int sbeg = pc[2], nstrs = pc[3], nprg, npats = pc[4], plen, len, i;
 	Patprog *pp;
 
-	pc += 5;
+	pc += 6;
 
 	nprg = end - pc;
 	plen = nprg * sizeof(wordcode);
diff --git a/Src/parse.c b/Src/parse.c
index 8c14aa80b..eb32d4faf 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -173,6 +173,7 @@ struct heredocs *hdocs;
  *     - followed by offset to first string
  *     - followed by length of string table
  *     - followed by number of patterns for body
+ *     - followed by a placeholder
  *     - followed by codes for body
  *     - followed by strings for body
  *     - if number of names is 0, followed by:
@@ -1691,6 +1692,7 @@ par_funcdef(int *cmplx)
     ecadd(0); /* p + num + 2 */
     ecadd(0); /* p + num + 3 */
     ecadd(0); /* p + num + 4 */
+    ecadd(0); /* p + num + 5 */
 
     nocorrect = 0;
     incmdpos = 1;
@@ -1730,6 +1732,7 @@ par_funcdef(int *cmplx)
     ecbuf[p + num + 2] = so - oecssub;
     ecbuf[p + num + 3] = ecsoffs - so; /* "length of string table" */
     ecbuf[p + num + 4] = ecnpats; /* "number of patterns for body" */
+    ecbuf[p + num + 5] = 0;
     ecbuf[p + 1] = num; /* "number of names" */
 
     ecnpats = onp;
@@ -2053,6 +2056,7 @@ par_simple(int *cmplx, int nr)
 	    ecadd(0);
 	    ecadd(0);
 	    ecadd(0);
+	    ecadd(0);
 
 	    ecnfunc++;
 	    ecssub = so = ecsoffs;
@@ -2108,6 +2112,7 @@ par_simple(int *cmplx, int nr)
 	    ecbuf[p + argc + 2] = so - oecssub;
 	    ecbuf[p + argc + 3] = ecsoffs - so;
 	    ecbuf[p + argc + 4] = ecnpats;
+	    ecbuf[p + argc + 5] = 0;
 
 	    ecnpats = onp;
 	    ecssub = oecssub;
diff --git a/Src/text.c b/Src/text.c
index 69530ae79..4bf88f2e2 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -600,7 +600,7 @@ gettext2(Estate state)
 		    n->u._funcdef.end = end;
 		    n->u._funcdef.nargs = nargs;
 		    state->strs += *state->pc;
-		    state->pc += 3;
+		    state->pc += 4;
 		}
 	    } else {
 		state->strs = s->u._funcdef.strs;

[-- Attachment #9: 0008-Add-the-function-T-syntax.patch.txt --]
[-- Type: text/plain, Size: 6248 bytes --]

From c7b30b127659fce57329dbf1b646021454d0f5e3 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Thu, 19 Mar 2020 18:00:16 +0000
Subject: [PATCH 8/9] Add the 'function -T' syntax.

Config/version.mk was bumped in the previous commit.
---
 Doc/Zsh/grammar.yo  | 13 ++++++++++++-
 README              |  7 +++++++
 Src/exec.c          |  8 +++++---
 Src/parse.c         | 11 +++++++++--
 Test/E02xtrace.ztst | 25 +++++++++++++++++++++++++
 5 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/Doc/Zsh/grammar.yo b/Doc/Zsh/grammar.yo
index e028c8512..fa0d72ff5 100644
--- a/Doc/Zsh/grammar.yo
+++ b/Doc/Zsh/grammar.yo
@@ -357,7 +357,7 @@ deliberately left unspecified, because historically there was a mismatch between
 the documented and implemented behaviours.  Cf. 20076, 21734/21735, 45075.)
 )
 findex(function)
-xitem(tt(function) var(word) ... [ tt(()) ] [ var(term) ] tt({) var(list) tt(}))
+xitem(tt(function) [ tt(-T) ] var(word) ... [ tt(()) ] [ var(term) ] tt({) var(list) tt(}))
 xitem(var(word) ... tt(()) [ var(term) ] tt({) var(list) tt(}))
 item(var(word) ... tt(()) [ var(term) ] var(command))(
 where var(term) is one or more newline or tt(;).
@@ -367,6 +367,17 @@ are usually only useful for setting traps.
 The body of the function is the var(list) between
 the tt({) and tt(}).  See noderef(Functions).
 
+The options of tt(function) have the following meanings:
+
+startitem()
+item(-T)(
+Enable tracing for this function, as though with tt(functions -T).  See the
+documentation of the tt(-f) option to the tt(typeset) builtin, in
+ifzman(zmanref(zshbuiltins))\
+ifnzman(noderef(Shell Builtin Commands)).
+)
+enditem()
+
 If the option tt(SH_GLOB) is set for compatibility with other shells, then
 whitespace may appear between the left and right parentheses when
 there is a single var(word);  otherwise, the parentheses will be treated as
diff --git a/README b/README
index 2bd5c2179..ae4f788bc 100644
--- a/README
+++ b/README
@@ -43,6 +43,13 @@ name of an external command.  Now it may also be a shell function.  Normal
 command word precedece rules apply, so if you have a function and a command
 with the same name, the function will be used.
 
+The syntax "function -T { ... }" used to define a function named "-T".
+It now defines an anonymous function with single-level tracing enabled ---
+same as "function f { ... }; functions -T f; f", but without naming the
+function.  The syntax "function -T foo { ... }" is similarly affected: it
+now defines a function "foo" with tracing enabled; previously it defined
+two functions, named "-T" and "foo" (see the MULTI_FUNC_DEF option).
+
 Incompatibilities since 5.7.1
 -----------------------------
 
diff --git a/Src/exec.c b/Src/exec.c
index 3c3fcfa3e..2b8e2167f 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5157,23 +5157,25 @@ execfuncdef(Estate state, Eprog redir_prog)
 {
     Shfunc shf;
     char *s = NULL;
-    int signum, nprg, sbeg, nstrs, npats, len, plen, i, htok = 0, ret = 0;
+    int signum, nprg, sbeg, nstrs, npats, do_tracing, len, plen, i, htok = 0, ret = 0;
     int anon_func = 0;
     Wordcode beg = state->pc, end;
     Eprog prog;
     Patprog *pp;
     LinkList names;
+    int tracing_flags;
 
     end = beg + WC_FUNCDEF_SKIP(state->pc[-1]);
     names = ecgetlist(state, *state->pc++, EC_DUPTOK, &htok);
     sbeg = *state->pc++;
     nstrs = *state->pc++;
     npats = *state->pc++;
-    (void) *state->pc++;
+    do_tracing = *state->pc++;
 
     nprg = (end - state->pc);
     plen = nprg * sizeof(wordcode);
     len = plen + (npats * sizeof(Patprog)) + nstrs;
+    tracing_flags = do_tracing ? PM_TAGGED_LOCAL : 0;
 
     if (htok && names) {
 	execsubst(names);
@@ -5223,7 +5225,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 
 	shf = (Shfunc) zalloc(sizeof(*shf));
 	shf->funcdef = prog;
-	shf->node.flags = 0;
+	shf->node.flags = tracing_flags;
 	/* No dircache here, not a directory */
 	shf->filename = ztrdup(scriptfilename);
 	shf->lineno =
diff --git a/Src/parse.c b/Src/parse.c
index eb32d4faf..dafc8cca6 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -173,7 +173,7 @@ struct heredocs *hdocs;
  *     - followed by offset to first string
  *     - followed by length of string table
  *     - followed by number of patterns for body
- *     - followed by a placeholder
+ *     - followed by an integer indicating tracing status
  *     - followed by codes for body
  *     - followed by strings for body
  *     - if number of names is 0, followed by:
@@ -1670,6 +1670,7 @@ par_funcdef(int *cmplx)
     int oecused = ecused, num = 0, onp, p, c = 0;
     int so, oecssub = ecssub;
     zlong oldlineno = lineno;
+    int do_tracing = 0;
 
     lineno = 0;
     nocorrect = 1;
@@ -1679,6 +1680,12 @@ par_funcdef(int *cmplx)
     p = ecadd(0);
     ecadd(0); /* p + 1 */
 
+    if (tok == STRING && tokstr[0] == Dash &&
+	tokstr[1] == 'T' && !tokstr[2]) {
+	++do_tracing;
+	zshlex();
+    }
+
     while (tok == STRING) {
 	if ((*tokstr == Inbrace || *tokstr == '{') &&
 	    !tokstr[1]) {
@@ -1732,7 +1739,7 @@ par_funcdef(int *cmplx)
     ecbuf[p + num + 2] = so - oecssub;
     ecbuf[p + num + 3] = ecsoffs - so; /* "length of string table" */
     ecbuf[p + num + 4] = ecnpats; /* "number of patterns for body" */
-    ecbuf[p + num + 5] = 0;
+    ecbuf[p + num + 5] = do_tracing;
     ecbuf[p + 1] = num; /* "number of names" */
 
     ecnpats = onp;
diff --git a/Test/E02xtrace.ztst b/Test/E02xtrace.ztst
index 795f7e616..d72b2d000 100644
--- a/Test/E02xtrace.ztst
+++ b/Test/E02xtrace.ztst
@@ -180,3 +180,28 @@
 >	# traced
 >	echo inner
 >}
+
+ function -T { echo traced anonymous function }
+ functions -- -T # no output
+1:define traced function: anonymous function
+?+(anon):0> echo traced anonymous function
+>traced anonymous function
+
+ function -T f { echo traced named function }
+ functions -- -T # no output
+ functions f
+ f
+0:define traced function: named function
+>f () {
+>	# traced
+>	echo traced named function
+>}
+?+f:0> echo traced named function
+>traced named function
+
+ function -T -T { echo trace function literally named "-T" }
+ -T
+0:define traced function: parse test
+?+-T:0> echo trace function literally named -T
+>trace function literally named -T
+

[-- Attachment #10: 0009-Add-end-of-options-guard-support-to-function-T.patch.txt --]
[-- Type: text/plain, Size: 3717 bytes --]

From 0a0e1d5d3191e2a2ef9217ce66c87de60cefa3bf Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Thu, 19 Mar 2020 18:11:39 +0000
Subject: [PATCH 9/9] Add end-of-options guard support to 'function -T'.

---
 README              | 23 +++++++++++++++++------
 Src/parse.c         | 16 ++++++++++++----
 Test/E02xtrace.ztst | 22 +++++++++++++++++++++-
 3 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/README b/README
index ae4f788bc..d08440ce1 100644
--- a/README
+++ b/README
@@ -43,12 +43,23 @@ name of an external command.  Now it may also be a shell function.  Normal
 command word precedece rules apply, so if you have a function and a command
 with the same name, the function will be used.
 
-The syntax "function -T { ... }" used to define a function named "-T".
-It now defines an anonymous function with single-level tracing enabled ---
-same as "function f { ... }; functions -T f; f", but without naming the
-function.  The syntax "function -T foo { ... }" is similarly affected: it
-now defines a function "foo" with tracing enabled; previously it defined
-two functions, named "-T" and "foo" (see the MULTI_FUNC_DEF option).
+The "function" reserved word, used to define functions, gained a new -T option.
+That affects syntaxes such as:
+
+1. "function -T { ... }".  It used to define a function named "-T".  It
+now defines and executes an anonymous function with single-level tracing
+enabled --- same as "function f { ... }; functions -T f; f", but without
+naming the function.
+
+2. "function -T foo { ... }".  It used to define two functions, named "-T"
+and "foo" (see the MULTI_FUNC_DEF option).  It now defines a function
+"foo" with tracing enabled.
+
+3. "function -- { ... }".  It used to define a function named "--".  It
+now defines and executes an anonymous function.  The "--" is taken to be
+an end-of-options guard (same as "ls --").
+
+The sh-compatible function definition syntax, "f() { ... }", is unchanged.
 
 Incompatibilities since 5.7.1
 -----------------------------
diff --git a/Src/parse.c b/Src/parse.c
index dafc8cca6..b3650526b 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1680,10 +1680,18 @@ par_funcdef(int *cmplx)
     p = ecadd(0);
     ecadd(0); /* p + 1 */
 
-    if (tok == STRING && tokstr[0] == Dash &&
-	tokstr[1] == 'T' && !tokstr[2]) {
-	++do_tracing;
-	zshlex();
+    /* Consume an initial (-T), (--), or (-T --).
+     * Anything else is a literal function name.
+     */
+    if (tok == STRING && tokstr[0] == Dash) {
+	if (tokstr[1] == 'T' && !tokstr[2]) {
+	    ++do_tracing;
+	    zshlex();
+	}
+	if (tok == STRING && tokstr[0] == Dash &&
+	    tokstr[1] == Dash && !tokstr[2]) {
+	    zshlex();
+	}
     }
 
     while (tok == STRING) {
diff --git a/Test/E02xtrace.ztst b/Test/E02xtrace.ztst
index d72b2d000..8b9cc89a8 100644
--- a/Test/E02xtrace.ztst
+++ b/Test/E02xtrace.ztst
@@ -199,9 +199,29 @@
 ?+f:0> echo traced named function
 >traced named function
 
- function -T -T { echo trace function literally named "-T" }
+ function -T -- -T { echo trace function literally named "-T" }
  -T
+ function -T -- { echo trace anonymous function }
+ functions -- -- # no output
 0:define traced function: parse test
 ?+-T:0> echo trace function literally named -T
 >trace function literally named -T
+?+(anon):0> echo trace anonymous function
+>trace anonymous function
+
+ function -- g { echo g }
+ g
+ function -- { echo anonymous }
+ functions -- -- # no output
+0:function end-of-"options" syntax, #1
+>g
+>anonymous
+
+ function -- -T { echo runs }
+ functions -- -- # no output
+ echo the definition didn\'t execute it
+ -T
+0:function end-of-"options" syntax, #2
+>the definition didn't execute it
+>runs
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-03-20  2:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18 18:30 RFC: function -T f { … } Daniel Shahaf
2020-03-18 19:53 ` Peter Stephenson
2020-03-20  2:04   ` Daniel Shahaf

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).