PATCH zsh/param/private and related stuff

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

I'm not sure whether one of the recent changes for warn_create_global
caused the problems noted in 37196 - 37198 or whether they were there
before and the [lack of] warnings just made it more obvious.  However,
there's an error routine in param_private.c that was supposed to be
called if a private was assigned at a nested scope, and that routine
never gets called now.  (At some point I may try git-bisect if I can
come up with a sufficient test case.)

In any event as you can see from the source comments I added, I finally
figured out that "exp" is Wischnowskese for "explicit" as the second
argument of the unset callbacks, so I was able to partially fix things
by referencing that.  Some work still needed RE the lack of call to the
aforementioned error routine.

There's some commented-out code in here that will probably turn into
warning messages in a subsequent patch.  I think we could safely put
this out as 5.2 with a mention in the BUGS file of the 37196 problem.

diff --git a/Src/Modules/param_private.c b/Src/Modules/param_private.c
index 7f9aa79..7d1ba9c 100644
--- a/Src/Modules/param_private.c
+++ b/Src/Modules/param_private.c
@@ -218,8 +218,10 @@ setfn_error(Param pm)
  * The unsetfn family compare locallevel and restore the old GSU before
  * calling the original unsetfn.  This assures that if the old unsetfn
  * wants to use its getfn or setfn, they're unconditionally present.
+ * The "explicit" flag indicates that "unset" was called, if zero the
+ * parameter is going out of scope (see params.c).
  *
- */ 
+ */
 
 /**/
 static char *
@@ -248,13 +250,15 @@ pps_setfn(Param pm, char *x)
 
 /**/
 static void
-pps_unsetfn(Param pm, int x)
+pps_unsetfn(Param pm, int explicit)
 {
     struct gsu_closure *c = (struct gsu_closure *)(pm->gsu.s);
     GsuScalar gsu = (GsuScalar)(c->g);
     pm->gsu.s = gsu;
     if (locallevel <= pm->level)
-	gsu->unsetfn(pm, x);
+	gsu->unsetfn(pm, explicit);
+    if (explicit)
+	pm->gsu.s = (GsuScalar)c;
 }
 
 /**/
@@ -283,13 +287,15 @@ ppi_setfn(Param pm, zlong x)
 
 /**/
 static void
-ppi_unsetfn(Param pm, int x)
+ppi_unsetfn(Param pm, int explicit)
 {
     struct gsu_closure *c = (struct gsu_closure *)(pm->gsu.i);
     GsuInteger gsu = (GsuInteger)(c->g);
     pm->gsu.i = gsu;
     if (locallevel <= pm->level)
-	gsu->unsetfn(pm, x);
+	gsu->unsetfn(pm, explicit);
+    if (explicit)
+	pm->gsu.i = (GsuInteger)c;
 }
 
 /**/
@@ -318,13 +324,15 @@ ppf_setfn(Param pm, double x)
 
 /**/
 static void
-ppf_unsetfn(Param pm, int x)
+ppf_unsetfn(Param pm, int explicit)
 {
     struct gsu_closure *c = (struct gsu_closure *)(pm->gsu.f);
     GsuFloat gsu = (GsuFloat)(c->g);
     pm->gsu.f = gsu;
     if (locallevel <= pm->level)
-	gsu->unsetfn(pm, x);
+	gsu->unsetfn(pm, explicit);
+    if (explicit)
+	pm->gsu.f = (GsuFloat)c;
 }
 
 /**/
@@ -354,13 +362,15 @@ ppa_setfn(Param pm, char **x)
 
 /**/
 static void
-ppa_unsetfn(Param pm, int x)
+ppa_unsetfn(Param pm, int explicit)
 {
     struct gsu_closure *c = (struct gsu_closure *)(pm->gsu.a);
     GsuArray gsu = (GsuArray)(c->g);
     pm->gsu.a = gsu;
     if (locallevel <= pm->level)
-	gsu->unsetfn(pm, x);
+	gsu->unsetfn(pm, explicit);
+    if (explicit)
+	pm->gsu.a = (GsuArray)c;
 }
 
 static HashTable emptytable;
@@ -391,13 +401,15 @@ pph_setfn(Param pm, HashTable x)
 
 /**/
 static void
-pph_unsetfn(Param pm, int x)
+pph_unsetfn(Param pm, int explicit)
 {
     struct gsu_closure *c = (struct gsu_closure *)(pm->gsu.h);
     GsuHash gsu = (GsuHash)(c->g);
     pm->gsu.h = gsu;
     if (locallevel <= pm->level)
-	gsu->unsetfn(pm, x);
+	gsu->unsetfn(pm, explicit);
+    if (explicit)
+	pm->gsu.h = (GsuHash)c;
 }
 
 /*
@@ -425,9 +437,13 @@ scopeprivate(HashNode hn, int onoff)
 		pm->node.flags |= PM_NORESTORE;
 	    else
 		pm->node.flags |= PM_UNSET;
-	else if (!(pm->node.flags & PM_NORESTORE))
-	    pm->node.flags &= ~PM_UNSET;
-	pm->node.flags &= ~PM_NORESTORE;
+	else {
+	    if (pm->node.flags & PM_NORESTORE)
+		pm->node.flags |= PM_UNSET;	/* createparam() may frob */
+	    else
+		pm->node.flags &= ~PM_UNSET;
+	    pm->node.flags &= ~PM_NORESTORE;
+	}
     }
 }
 
diff --git a/Src/params.c b/Src/params.c
index 602f69f..4600284 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -880,6 +880,10 @@ createparam(char *name, int flags)
 		zerr("read-only variable: %s", name);
 		return NULL;
 	    }
+	    if ((oldpm->node.flags & PM_RESTRICTED) && isset(RESTRICTED)) {
+		zerr("%s: restricted", name);
+		return NULL;
+	    }
 	    if (!(oldpm->node.flags & PM_UNSET) || (oldpm->node.flags & PM_SPECIAL)) {
 		oldpm->node.flags &= ~PM_UNSET;
 		if ((oldpm->node.flags & PM_SPECIAL) && oldpm->ename) {
@@ -890,10 +894,6 @@ createparam(char *name, int flags)
 		}
 		return NULL;
 	    }
-	    if ((oldpm->node.flags & PM_RESTRICTED) && isset(RESTRICTED)) {
-		zerr("%s: restricted", name);
-		return NULL;
-	    }
 
 	    pm = oldpm;
 	    pm->base = pm->width = 0;
@@ -2777,6 +2777,7 @@ assignsparam(char *s, char *val, int flags)
     if (!v && !(v = getvalue(&vbuf, &t, 1))) {
 	unqueue_signals();
 	zsfree(val);
+	/* errflag |= ERRFLAG_ERROR; */
 	return NULL;
     }
     if (flags & ASSPM_WARN_CREATE)
@@ -2926,6 +2927,7 @@ assignaparam(char *s, char **val, int flags)
 	if (!(v = fetchvalue(&vbuf, &t, 1, SCANPM_ASSIGNING))) {
 	    unqueue_signals();
 	    freearray(val);
+	    /* errflag |= ERRFLAG_ERROR; */
 	    return NULL;
 	}
 
@@ -2988,6 +2990,7 @@ sethparam(char *s, char **val)
 	return NULL;
     queue_signals();
     if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) {
+	DPUTS(!v, "BUG: assigning to undeclared associative array");
 	createparam(t, PM_HASHED);
 	checkcreate = isset(WARNCREATEGLOBAL) && locallevel > 0;
     } else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) &&
@@ -3000,6 +3003,7 @@ sethparam(char *s, char **val)
     if (!v)
 	if (!(v = fetchvalue(&vbuf, &t, 1, SCANPM_ASSIGNING))) {
 	    unqueue_signals();
+	    /* errflag |= ERRFLAG_ERROR; */
 	    return NULL;
 	}
     if (checkcreate)
@@ -3061,8 +3065,11 @@ setnparam(char *s, mnumber val)
 	} else if (val.type & MN_INTEGER) {
 	    pm->base = outputradix;
 	}
-	v = getvalue(&vbuf, &t, 1);
-	DPUTS(!v, "BUG: value not found for new parameter");
+	if (!(v = getvalue(&vbuf, &t, 1))) {
+	    DPUTS(!v, "BUG: value not found for new parameter");
+	    /* errflag |= ERRFLAG_ERROR; */
+	    return NULL;
+	}
 	if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0)
 	    check_warn_create(v->pm, "numeric");
     }
@@ -3219,6 +3226,10 @@ unsetparam_pm(Param pm, int altflag, int exp)
  *
  * This could usefully be made type-specific, but then we need
  * to be more careful when calling the unset method directly.
+ *
+ * The "exp"licit parameter should be nonzero for assignments and the
+ * unset command, and zero for implicit unset (e.g., end of scope).
+ * Currently this is used only by some modules.
  */
 
 /**/
@@ -5105,8 +5116,11 @@ freeparamnode(HashNode hn)
 {
     Param pm = (Param) hn;
  
-    /* Since the second flag to unsetfn isn't used, I don't *
-     * know what its value should be.                       */
+    /* The second argument of unsetfn() is used by modules to
+     * differentiate "exp"licit unset from implicit unset, as when
+     * a parameter is going out of scope.  It's not clear which
+     * of these applies here, but passing 1 has always worked.
+     */
     if (delunset)
 	pm->gsu.s->unsetfn(pm, 1);
     zsfree(pm->node.nam);
diff --git a/Test/V10private.ztst b/Test/V10private.ztst
index c66f175..f877455 100644
--- a/Test/V10private.ztst
+++ b/Test/V10private.ztst
@@ -11,6 +11,11 @@
  (zmodload -u zsh/param/private && zmodload zsh/param/private)
 0:unload and reload the module without crashing
 
+ $ZTST_exe +Z -f $ZTST_srcdir/ztst.zsh $ZTST_srcdir/B02typeset.ztst
+0:typeset still works with zsh/param/private module loaded
+*>*
+*>*
+
  typeset scalar_test=toplevel
  () {
   print $scalar_test
@@ -237,7 +242,7 @@ F:note "typeset" rather than "private" in output from outer
   }
   print Y ${(kv)hash_test} Z $array_test
  }
- print ${(kv)hash_test}
+ print ${(kv)hash_test} ${(t)array_test}
 0:privates are not visible in anonymous functions, part 3
 >X top level
 >Y in function Z in function

-- 
Barton E. Schaefer