Re: [PATCH] Fix (mainly nameref) issues in builtin "unset"

[Philippe Altherr <philippe.altherr@xxxxxxxxx>]

I synced a few days ago but forgot to send an updated patch. I resynced a few minutes and here is the latest version.

- Fix nameref issues in builtin "unset"

I see that there are already a few more of my patches that were committed. Hopefully there is no conflict with these. I will check and send a new version if needed.

Philippe

diff --git a/Src/builtin.c b/Src/builtin.c
index 96bfd64cb..0ae75e6bd 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3841,10 +3841,9 @@ bin_unset(char *name, char **argv, Options ops, int func)
 			/* record pointer to next, since we may free this one */
 			next = (Param) pm->node.next;
 			if (pattry(pprog, pm->node.nam)) {
-			    if (!OPT_ISSET(ops,'n') &&
-				(pm->node.flags & PM_NAMEREF) && pm->u.str)
-				unsetparam(pm->u.str);
-			    else
+			    if (OPT_ISSET(ops,'n') ||
+				((pm = resolve_nameref(pm)) &&
+				 !(pm->node.flags & PM_NAMEREF)))
 				unsetparam_pm(pm, 0, 1);
 			    match++;
 			}
@@ -3937,22 +3936,11 @@ bin_unset(char *name, char **argv, Options ops, int func)
 		zerrnam(name, "%s: invalid element for unset", s);
 		returnval = 1;
 	    }
-	} else {
-	    if (!OPT_ISSET(ops,'n')) {
-		int ref = (pm->node.flags & PM_NAMEREF);
-		if (!(pm = resolve_nameref(pm)))
-		    continue;
-		if (ref && pm->level < locallevel &&
-		    !(pm->node.flags & PM_READONLY)) {
-		    /* Just mark unset, do not remove from table */
-		    stdunsetfn(pm, 0);
-		    pm->node.flags |= PM_DECLARED;
-		    continue;
-		}
-	    }
+	} else if (OPT_ISSET(ops,'n') ||
+		   ((pm = resolve_nameref(pm)) &&
+		    !(pm->node.flags & PM_NAMEREF)))
 	    if (unsetparam_pm(pm, 0, 1))
 		returnval = 1;
-	}
 	if (ss)
 	    *ss = '[';
     }
diff --git a/Src/params.c b/Src/params.c
index 68d4c0bc3..b95efe9a8 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -499,6 +499,16 @@ static Param argvparam;
  * times in the same list. Non of that is harmful as long as only
  * instances that are still references referring to the ending scope
  * are updated when the scope ends.
+ *
+ * The list corresponding to the global scope never receives any of
+ * the named references described above. Instead, it's used to track
+ * global parameters that were unset via a named reference while in a
+ * scope where they were hidden by a nested parameter with the same
+ * name. In such cases, the global parameter's Param instance can't be
+ * deleted as usual. Instead, it's marked as unset and added to the
+ * global scope's list. Each time a scope ends, the list is traversed
+ * and parameters that are still unset but no longer hidden are
+ * deleted.
  */
 static LinkList *scoperefs = NULL;
 static int scoperefs_num = 0;
@@ -3904,6 +3914,24 @@ unsetparam_pm(Param pm, int altflag, int exp)
 	(pm->node.flags & (PM_SPECIAL|PM_REMOVABLE)) == PM_SPECIAL)
 	return 0;
 
+    /*
+     * Global variables can only be deleted if they aren't hidden by a
+     * local one with the same name.
+     */
+    if (!pm->level &&
+	pm != (Param) (paramtab == realparamtab ?
+		       /* getnode2() to avoid autoloading */
+		       paramtab->getnode2(paramtab, pm->node.nam) :
+		       paramtab->getnode(paramtab, pm->node.nam))) {
+	LinkList refs;
+	if (!scoperefs)
+	    scoperefs = zshcalloc((scoperefs_num = 8) * sizeof(refs));
+	if (!scoperefs[0])
+	    scoperefs[0] = znewlinklist();
+	zpushnode(scoperefs[0], pm);
+	return 0;
+    }
+
     /* remove parameter node from table */
     paramtab->removenode(paramtab, pm->node.nam);
 
@@ -5878,6 +5906,15 @@ endparamscope(void)
 	    setscope(pm);
 	}
     }
+    /* Delete unset global variables that were hidden at unset time */
+    if ((refs = scoperefs ? scoperefs[0] : NULL)) {
+	scoperefs[0] = NULL;
+	for (Param pm; refs && (pm = (Param)getlinknode(refs));) {
+	    if ((pm->node.flags & PM_UNSET) && !(pm->node.flags & PM_DECLARED))
+		unsetparam_pm(pm, 1, 0);
+	}
+	freelinklist(refs, NULL);
+    }
     unqueue_signals();
 }
 
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index 77a01ea48..68b36f415 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -1026,15 +1026,15 @@ F:Checking for a bug in zmodload that affects later tests
  typeset -p .K01.{scalar,assoc,array,integer,double,float,readonly}
  unset .K01.{scalar,assoc,array,integer,double,float}
 0:unset various types via nameref, including a readonly special
->typeset -g .K01.scalar
->typeset -g -A .K01.assoc
->typeset -g -a .K01.array
->typeset -g -i .K01.integer
->typeset -g -E .K01.double
->typeset -g -F .K01.float
 >typeset -g -r .K01.readonly=RO
 *?*read-only variable: ARGC
 *?*read-only variable: .K01.readonly
+*?*no such variable: .K01.scalar
+*?*no such variable: .K01.assoc
+*?*no such variable: .K01.array
+*?*no such variable: .K01.integer
+*?*no such variable: .K01.double
+*?*no such variable: .K01.float
 
  unset -n ref
  unset one
@@ -1946,6 +1946,220 @@ F:converting from association/array to string should work here too
 ># d:reference to not-yet-defined - local - ref1
 >typeset -i var=42
 
+ test-unset() {
+   typeset var0=foo
+   typeset -n ref1=var0 ref2=ref1
+   typeset cmd=(unset $@); echo "#" $cmd; $cmd
+   typeset -p var0 ref1 ref2
+ }
+ test-unset -n    ref1
+ test-unset -n    ref2
+ test-unset -n -m ref1
+ test-unset -n -m ref2
+ unfunction test-unset
+0:unsetting references with -n unsets the references
+># unset -n ref1
+>typeset var0=foo
+>typeset -n ref2=ref1
+># unset -n ref2
+>typeset var0=foo
+>typeset -n ref1=var0
+># unset -n -m ref1
+>typeset var0=foo
+>typeset -n ref2=ref1
+># unset -n -m ref2
+>typeset var0=foo
+>typeset -n ref1=var0
+
+ test-unset() {
+   typeset var0=foo
+   typeset -n ref1=var0 ref2=ref1
+   typeset cmd=(unset $@); echo "#" $cmd; $cmd
+   typeset -p var0 ref1 ref2
+ }
+ test-unset    ref1
+ test-unset    ref2
+ test-unset -m ref1
+ test-unset -m ref2
+ unfunction test-unset
+0:unsetting references without -n unsets the referred parameters
+># unset ref1
+>typeset -n ref1=var0
+>typeset -n ref2=ref1
+># unset ref2
+>typeset -n ref1=var0
+>typeset -n ref2=ref1
+># unset -m ref1
+>typeset -n ref1=var0
+>typeset -n ref2=ref1
+># unset -m ref2
+>typeset -n ref1=var0
+>typeset -n ref2=ref1
+
+ test-unset() {
+   typeset var0=12345
+   typeset -n ref1=var0 ref2=ref1
+   typeset cmd=(unset $@); echo "#" $cmd; $cmd
+   typeset -p var0
+ }
+ test-unset    ref1"[3]"
+ test-unset    ref2"[3]"
+ test-unset -n ref1"[3]"
+ test-unset -n ref2"[3]"
+ unfunction test-unset
+0:unsetting subscripted references unsets the referred elements
+># unset ref1[3]
+>typeset var0=1245
+># unset ref2[3]
+>typeset var0=1245
+># unset -n ref1[3]
+>typeset var0=1245
+># unset -n ref2[3]
+>typeset var0=1245
+
+ test-unset() {
+   typeset -r var=foo
+   typeset -n ref=var
+   typeset cmd=(unset $@); echo "#" $cmd; { $cmd 2>&1 } always { TRY_BLOCK_ERROR=0 }
+   typeset -p var
+ }
+ test-unset    var
+ test-unset -m var
+ test-unset    ref
+ test-unset -m ref
+ test-unset    var"[2]"
+ test-unset    ref"[2]"
+ test-unset -n ref"[2]"
+ unfunction test-unset
+0:unsetting read-only parameter triggers an error
+># unset var
+>test-unset:3: read-only variable: var
+>typeset -r var=foo
+># unset -m var
+>test-unset:3: read-only variable: var
+>typeset -r var=foo
+># unset ref
+>test-unset:3: read-only variable: var
+>typeset -r var=foo
+># unset -m ref
+>test-unset:3: read-only variable: var
+>typeset -r var=foo
+># unset var[2]
+>test-unset:3: read-only variable: var
+>typeset -r var=foo
+># unset ref[2]
+>test-unset:3: read-only variable: var
+>typeset -r var=foo
+># unset -n ref[2]
+>test-unset:3: read-only variable: var
+>typeset -r var=foo
+
+ test-unset() {
+   typeset -n ref1 ref2=ref1
+   typeset cmd=(unset $@); echo "#" $cmd; $cmd
+   typeset -p ref1 ref2
+ }
+ test-unset    ref1
+ test-unset    ref2
+ test-unset -m ref1
+ test-unset -m ref2
+ unfunction test-unset
+0:unsetting placeholder references or their referents has no effect
+># unset ref1
+>typeset -n ref1
+>typeset -n ref2=ref1
+># unset ref2
+>typeset -n ref1
+>typeset -n ref2=ref1
+># unset -m ref1
+>typeset -n ref1
+>typeset -n ref2=ref1
+># unset -m ref2
+>typeset -n ref1
+>typeset -n ref2=ref1
+
+ test-unset() {
+   typeset -n ref1=undefined ref2=ref1
+   typeset cmd=(unset $@); echo "#" $cmd; $cmd
+   typeset -p ref1 ref2
+ }
+ typeset -p undefined 2>&1
+ test-unset    ref1
+ test-unset    ref2
+ test-unset -m ref1
+ test-unset -m ref2
+ unfunction test-unset
+0:unsetting references to not-yet-defined variables or their referents has no effect
+>(eval):typeset:6: no such variable: undefined
+># unset ref1
+>typeset -n ref1=undefined
+>typeset -n ref2=ref1
+># unset ref2
+>typeset -n ref1=undefined
+>typeset -n ref2=ref1
+># unset -m ref1
+>typeset -n ref1=undefined
+>typeset -n ref2=ref1
+># unset -m ref2
+>typeset -n ref1=undefined
+>typeset -n ref2=ref1
+
+ test-unset() {
+   typeset -n refg1=g1 refl1=l1
+   () {
+     typeset -g g1=glb1 g2=glb2
+     typeset    l1=lcl1 l2=lcl2
+     () {
+       typeset -n refg2=g2 refl2=l2
+       typeset cmd=(unset $@ refg1 refg2 refl1 refl2); echo "#" $cmd; $cmd
+     } $@
+     typeset -p g1 g2 l1 l2 2>&1
+   } $@
+   unset g1 g2
+ }
+ test-unset
+ test-unset -m
+ unfunction test-unset
+0:unsetting references referring to parameters in enclosing scopes unsets the parameters
+># unset refg1 refg2 refl1 refl2
+>(anon):typeset:7: no such variable: g1
+>(anon):typeset:7: no such variable: g2
+># unset -m refg1 refg2 refl1 refl2
+>(anon):typeset:7: no such variable: g1
+>(anon):typeset:7: no such variable: g2
+
+ test-unset() {
+   typeset -g g=glb
+   typeset    l=lcl
+   typeset -n refg=g refl=l
+   () {
+     typeset g=hide-g
+     typeset l=hide-l
+     typeset cmd=(unset $@ refg refl); echo "#" $cmd; $cmd
+     echo "# inner scope"
+     typeset -p g l 2>&1
+   } $@
+   echo "# outer scope"
+   typeset -p g l 2>&1
+   unset g
+ }
+ test-unset
+ test-unset -m
+ unfunction test-unset
+0:unsetting references referring to hidden parameters unsets the hidden parameters
+># unset refg refl
+># inner scope
+>typeset g=hide-g
+>typeset l=hide-l
+># outer scope
+>test-unset:typeset:12: no such variable: g
+># unset -m refg refl
+># inner scope
+>typeset g=hide-g
+>typeset l=hide-l
+># outer scope
+>test-unset:typeset:12: no such variable: g
+
  typeset -n ref1
  typeset -n ref2
  typeset -n ref3=ref2