[PATCH] "typeset -nu" (was Re: Up-scope named references, vs. ksh)

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Fri, Mar 1, 2024 at 11:29 PM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> How about this:  Instead of magically turning a positional into a
> reference into the surrounding scope, let's make it explicit.

Here's a patch to implement this, including doc with an example.

Of particular note is the last sentence here:

+To force a named reference to refer to the outer scope, even if a local
+has already been declared, add the tt(-u) option when declaring the
+named reference.  In this case var(rname) should already exist in the
+outer scope, otherwise the behavior of assignment through var(pname)
+is not defined and may change the scope of the reference or fail with
+a status of 1.

I went with assignment silently failing rather than printing an error
message in the cases where a parameter in the calling scope cannot be
created.

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 6318053d8..7a9684ac8 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -2052,13 +2052,17 @@ cindex(named reference)
 cindex(reference, named)
 The flag tt(-n) creates a em(named reference) to another parameter.
 The second parameter need not exist at the time the reference is
-created.  Only tt(-g) and tt(-r) may be used in conjunction with
+created.  Only tt(-g), tt(-u), and tt(-r) may be used in conjunction with
 tt(-n).  The var(name) so created may not be an array element nor use
 a subscript, but the var(value) assigned may be any valid parameter
 name syntax, even a subscripted array element (including an associative
 array element) or an array slice, which is evaluated when the named
 reference is expanded.  It is an error for a named reference to refer
-to itself, even indirectly through a chain of references.
+to itself, even indirectly through a chain of references.  When tt(-u)
+is applied to a named reference, the parameter identified by var(value)
+is always found in the calling function scope rather than the current
+local scope.  In this case, if there is no such parameter in the calling
+scope, assignments to the named reference may fail, setting tt($?) to 1.
 See ifzman(zmanref(zshexpn))ifnzman(noderef(Parameter Expansion)) and
 ifzman(zmanref(zshparam))ifnzman(noderef(Parameters)) for details of the
 behavior of named references.
diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index 2acfd08c9..183ca6e03 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -1600,6 +1600,25 @@ example(tt(before local: OUTER)
 tt(by reference: OUTER)
 tt(after func: RESULT))
 
+To force a named reference to refer to the outer scope, even if a local
+has already been declared, add the tt(-u) option when declaring the
+named reference.  In this case var(rname) should already exist in the
+outer scope, otherwise the behavior of assignment through var(pname)
+is not defined and may change the scope of the reference or fail with
+a status of 1.  Example of correct usage:
+ifzman()
+example(tt(caller=OUTER)
+tt(func LPAR()RPAR() {)
+tt(  print before local: $caller)
+tt(  local caller=INNER)
+tt(  print after local: $caller)
+tt(  typeset -n -u outer=$1)
+tt(  print by reference: $outer)
+tt(  outer=RESULT)
+tt(})
+tt(func caller)
+tt(print after func: $caller))
+
 Note, however, that named references to em(special) parameters acquire
 the behavior of the special parameter, regardless of the scope where
 the reference is declared.
diff --git a/Doc/Zsh/func.yo b/Doc/Zsh/func.yo
index d4914df7a..7b71e34e9 100644
--- a/Doc/Zsh/func.yo
+++ b/Doc/Zsh/func.yo
@@ -31,10 +31,12 @@ referent parameter is in scope, and as early as possible in the
 function if the reference is to a parameter in a calling scope.
 
 A typical use of named references is to pass the name
-of the referent as a positional parameter.  For example,
+of the referent as a positional parameter.  In this case it is
+good practice to use the tt(-u) option to reference the calling
+scope.  For example,
 ifzman()
 example(pop+LPAR()RPAR() {
-  local -n ref=$1
+  local -nu ref=$1
   local last=$ref[$#ref]
   ref[$#ref]=LPAR()RPAR()
   print -r -- $last
@@ -43,9 +45,10 @@ array=LPAR() a list of five values RPAR()
 pop array)
 
 prints the word `tt(values)' and shortens `tt($array)' to
-`tt(LPAR() a list of five RPAR())'.  There are no local parameters in
-tt(pop) at the time `tt(ref=$1)' is assigned, so `tt(ref)' becomes a
-reference to `tt(array)' in the caller.
+`tt(LPAR() a list of five RPAR())'.  With tt(-nu), `tt(ref)' becomes a
+reference to `tt(array)' in the caller.  There are no local parameters in
+tt(pop) at the time `tt(ref=$1)' is assigned, so in this example tt(-u)
+could have been omitted, but it makes the intention clear.
 
 Functions execute in the same process as the caller and
 share all files
diff --git a/Doc/Zsh/mod_ksh93.yo b/Doc/Zsh/mod_ksh93.yo
index 9cd708d10..7508758aa 100644
--- a/Doc/Zsh/mod_ksh93.yo
+++ b/Doc/Zsh/mod_ksh93.yo
@@ -12,7 +12,7 @@ The single builtin provided by this module is:
 startitem()
 findex(nameref)
 cindex(named references, creating)
-item(tt(nameref) [ tt(-r) ] var(pname)[tt(=)var(rname)])(
+item(tt(nameref) [ tt(-gur) ] var(pname)[tt(=)var(rname)])(
 Equivalent to tt(typeset -n )var(pname)tt(=)var(rname)
 
 However, tt(nameref) is a builtin command rather than a reserved word,
diff --git a/Etc/FAQ.yo b/Etc/FAQ.yo
index 145ef02c9..4a86050e6 100644
--- a/Etc/FAQ.yo
+++ b/Etc/FAQ.yo
@@ -1025,6 +1025,13 @@ label(210)
     HIT:SPOT
   )
 
+  Dynamic scoping applies to named references, so for example a named
+  reference declared in global scope may be used in function scopes.
+  In ksh, local parameters have static scope, so named references in
+  zsh may have side-effects that do not occur in ksh.  To limit those
+  effects, mytt(zmodload zsh/param/private) and declare all named
+  references mytt(private).
+
   Named references may be used in zsh versions later than 5.9.
 
 sect(What is zsh's support for non-forking command substitution?)
diff --git a/Src/Modules/ksh93.c b/Src/Modules/ksh93.c
index 9af5e1d69..6760cbca0 100644
--- a/Src/Modules/ksh93.c
+++ b/Src/Modules/ksh93.c
@@ -38,7 +38,7 @@
  */
 
 static struct builtin bintab[] = {
-    BUILTIN("nameref", BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "gr", "n")
+    BUILTIN("nameref", BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "gur", "n")
 };
 
 #include "zsh.mdh"
diff --git a/Src/builtin.c b/Src/builtin.c
index 83144677b..ba9cb03c2 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2699,7 +2699,7 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 	    off |= bit;
     }
     if (OPT_MINUS(ops,'n')) {
-	if ((on|off) & ~PM_READONLY) {
+	if ((on|off) & ~(PM_READONLY|PM_UPPER)) {
 	    zwarnnam(name, "no other attributes allowed with -n");
 	    return 1;
 	}
diff --git a/Src/exec.c b/Src/exec.c
index d85adbea5..8be7bf17d 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1322,7 +1322,7 @@ execsimple(Estate state)
 	    fputc('\n', xtrerr);
 	    fflush(xtrerr);
 	}
-	lv = (errflag ? errflag : cmdoutval);
+	lv = (errflag ? errflag : cmdoutval ? cmdoutval : lastval);
     } else {
 	int q = queue_signal_level();
 	dont_queue_signals();
@@ -2604,6 +2604,7 @@ addvars(Estate state, Wordcode pc, int addflags)
 		opts[ALLEXPORT] = allexp;
 	    } else
 	    	pm = assignsparam(name, val, myflags);
+	    lastval = !pm; /* zerr("%s: assignment failed", name); */
 	    if (errflag) {
 		state->pc = opc;
 		return;
@@ -2628,7 +2629,9 @@ addvars(Estate state, Wordcode pc, int addflags)
 	    }
 	    fprintf(xtrerr, ") ");
 	}
-	assignaparam(name, arr, myflags);
+	;
+	lastval = !assignaparam(name, arr, myflags);
+	/* zerr("%s: array assignment failed", name); */
 	if (errflag) {
 	    state->pc = opc;
 	    return;
diff --git a/Src/params.c b/Src/params.c
index 064dbd2bc..790c978f2 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1060,8 +1060,7 @@ createparam(char *name, int flags)
 			  "BUG: local parameter is not unset");
 		    oldpm = lastpm;
 		}
-	    } else
-		flags |= PM_NAMEREF;
+	    }
 	}
 
 	DPUTS(oldpm && oldpm->level > locallevel,
@@ -6264,10 +6263,12 @@ resolve_nameref(Param pm, const Asgment stop)
 	    }
 	} else if ((hn = gethashnode2(realparamtab, seek))) {
 	    if (pm) {
-		if (!(stop && (stop->flags & (PM_LOCAL))))
-		    hn = (HashNode)upscope((Param)hn,
-					   ((pm->node.flags & PM_NAMEREF) ?
-					    pm->base : ((Param)hn)->level));
+		if (!(stop && (stop->flags & (PM_LOCAL)))) {
+		    int scope = ((pm->node.flags & PM_NAMEREF) ?
+				 ((pm->node.flags & PM_UPPER) ? -1 :
+				  pm->base) : ((Param)hn)->level);
+		    hn = (HashNode)upscope((Param)hn, scope);
+		}
 		/* user can't tag a nameref, safe for loop detection */
 		pm->node.flags |= PM_TAGGED;
 	    }
@@ -6313,11 +6314,13 @@ setloopvar(char *name, char *value)
 static void
 setscope(Param pm)
 {
-    if (pm->node.flags & PM_NAMEREF) {
+    queue_signals();
+    if (pm->node.flags & PM_NAMEREF) do {
 	Param basepm;
 	struct asgment stop;
 	char *refname = GETREFNAME(pm);
 	char *t = refname ? itype_end(refname, INAMESPC, 0) : NULL;
+	int q = queue_signal_level();
 
 	/* Temporarily change nameref to array parameter itself */
 	if (t && *t == '[')
@@ -6327,9 +6330,11 @@ setscope(Param pm)
 	stop.name = "";
 	stop.value.scalar = NULL;
 	stop.flags = PM_NAMEREF;
-	if (locallevel)
+	if (locallevel && !(pm->node.flags & PM_UPPER))
 	    stop.flags |= PM_LOCAL;
+	dont_queue_signals();	/* Prevent unkillable loops */
 	basepm = (Param)resolve_nameref(pm, &stop);
+	restore_queue_signals(q);
 	if (t) {
 	    pm->width = t - refname;
 	    *t = '[';
@@ -6342,7 +6347,7 @@ setscope(Param pm)
 			if (upscope(pm, pm->base) == pm) {
 			    zerr("%s: invalid self reference", refname);
 			    unsetparam_pm(pm, 0, 1);
-			    return;
+			    break;
 			}
 			pm->node.flags &= ~PM_SELFREF;
 		    } else if (pm->base == pm->level) {
@@ -6350,7 +6355,7 @@ setscope(Param pm)
 			    strcmp(pm->node.nam, refname) == 0) {
 			    zerr("%s: invalid self reference", refname);
 			    unsetparam_pm(pm, 0, 1);
-			    return;
+			    break;
 			}
 		    }
 		} else if ((t = GETREFNAME(basepm))) {
@@ -6358,7 +6363,7 @@ setscope(Param pm)
 			strcmp(pm->node.nam, t) == 0) {
 			zerr("%s: invalid self reference", refname);
 			unsetparam_pm(pm, 0, 1);
-			return;
+			break;
 		    }
 		}
 	    } else
@@ -6378,7 +6383,8 @@ setscope(Param pm)
 	    zerr("%s: invalid self reference", refname);
 	    unsetparam_pm(pm, 0, 1);
 	}
-    }
+    } while (0);
+    unqueue_signals();
 }
 
 /**/
@@ -6390,6 +6396,8 @@ upscope(Param pm, int reflevel)
 	pm = up;
 	up = up->old;
     }
+    if (reflevel < 0 && locallevel > 0)
+	return pm->level == locallevel ? up : pm;
     return pm;
 }
 
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index ff48e2289..6db819389 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -492,7 +492,6 @@ F:unexpected side-effects of previous tests
  }
  typeset -p ptr2
 1:up-reference part 5, stacked namerefs, end not in scope
-F:What is the correct behavior for the scope of ptr1?
 >typeset -n ptr1=ptr2
 >typeset -n ptr2
 >ptr1=ptr2
@@ -529,6 +528,49 @@ F:Same test, should part 5 output look like this?
 >typeset -n ptr2
 >typeset ptr2=val
 
+ () {
+   () {
+     local var
+     typeset -nu ptr1=var
+     ptr1=outer && print -u2 assignment expected to fail
+     typeset -n ptr2=var
+     ptr2=inner
+     typeset -n
+     printf "%s=%s\n" ptr1 "$ptr1" ptr2 "$ptr2"
+   }
+   typeset -p var
+ }
+ typeset -p var
+1:up-reference part 7, upscope namerefs, end not in scope
+>ptr1=var
+>ptr2=var
+>ptr1=
+>ptr2=inner
+*?*typeset*: no such variable: var
+?*typeset*: no such variable: var
+
+ typeset var
+ () {
+   () {
+     local var
+     typeset -nu ptr1=var
+     ptr1=outer || print -u2 assignment expected to succeed
+     typeset -n ptr2=var
+     ptr2=inner
+     typeset -n
+     printf "%s=%s\n" ptr1 "$ptr1" ptr2 "$ptr2"
+   }
+   typeset -p var
+ }
+ typeset -p var
+0:up-reference part 8, upscope namerefs, end in scope
+>ptr1=var
+>ptr2=var
+>ptr1=outer
+>ptr2=inner
+>typeset -g var=outer
+>typeset var=outer
+
  if zmodload zsh/parameter; then
  () {
    zmodload -u zsh/parameter
@@ -539,7 +581,7 @@ F:Same test, should part 5 output look like this?
  }
  else ZTST_skip='Cannot zmodload zsh/parameter, skipping autoload test'
  fi
-0:up-reference part 3, autoloading with hidden special
+0:up-reference part 9, autoloading with hidden special
 >nameref-local-nameref-local
 >typeset -h parameters
 
@@ -762,12 +804,17 @@ F:relies on global TYPESET_TO_UNSET in %prep
 
  bar=xx
  typeset -n foo=bar
- () { typeset -n foo; foo=zz; foo=zz; print $bar $zz }
+ () {
+   typeset -n foo; foo=zz
+   foo=zz || print -u2 foo: assignment failed
+   print $bar $zz
+ }
  () { typeset -n foo; foo=zz; local zz; foo=zz; print $bar $zz }
 0:regression: local nameref may not in-scope a global parameter
 F:previously this could create an infinite recursion and crash
 >xx
 >xx zz
+*?*foo: assignment failed
 
  typeset -nm foo=bar
 1:create nameref by pattern match not allowed
diff --git a/Test/V10private.ztst b/Test/V10private.ztst
index efa346002..ed51316f3 100644
--- a/Test/V10private.ztst
+++ b/Test/V10private.ztst
@@ -378,7 +378,7 @@ F:Here ptr1 finds private ptr2 by scope mismatch
    typeset -p ptr1 ptr2
    typeset val=LOCAL
    () {
-     ptr1=val		# This is a silent no-op, why?
+     ptr1=val || print -u2 ptr1: assignment failed
      typeset -n
      printf "%s=%s\n" ptr1 "$ptr1" ptr2 "$ptr2"
    }
@@ -388,7 +388,6 @@ F:Here ptr1 finds private ptr2 by scope mismatch
 1:up-reference for private namerefs, end not in scope
 F:See K01nameref.ztst up-reference part 5
 F:Here ptr1 finds private ptr2 by scope mismatch
-F:Assignment silently fails, is that correct?
 >typeset -n ptr1=ptr2
 >typeset -hn ptr2=''
 >ptr1=ptr2
@@ -396,6 +395,7 @@ F:Assignment silently fails, is that correct?
 >ptr2=
 >typeset -n ptr1=ptr2
 >typeset -hn ptr2=''
+*?*ptr1: assignment failed
 *?*no such variable: ptr2
 
  typeset ptr2