[PATCH] Declaring the same variable "private" more than once

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

Given that it's relatively harmless to do something like

  for x in *
  do
    local y=$x:r
    # etc.
  done

It occurred to me that it should similarly be possible to do that with
"local -P" or "private", but without the appended patch that generates
a warning message and leaves $y unchanged without aborting the loop.

With the below it remains an error to try to change the type of a
private parameter (and that is an actual error rather than just a
warning, like most other typeset errors) but simply reassigning
something of the same type is allowed.

diff --git a/Src/Modules/param_private.c b/Src/Modules/param_private.c
index 8e04b2b95..7ef6633da 100644
--- a/Src/Modules/param_private.c
+++ b/Src/Modules/param_private.c
@@ -87,9 +87,52 @@ makeprivate(HashNode hn, UNUSED(int flags))
 	      ((pm->node.flags & (PM_SPECIAL|PM_REMOVABLE)) == PM_SPECIAL &&
 	       /* typeset_single() line 2300 discards PM_REMOVABLE -- why? */
 	       !is_private(pm->old))))) {
-	    zwarnnam("private", "can't change scope of existing param: %s",
-		     pm->node.nam);
-	    makeprivate_error = 1;
+	    if (is_private(pm->old)) {
+		if (pm->old->node.flags & PM_READONLY) {
+		    zerr("read-only variable: %s", pm->node.nam);
+		    makeprivate_error = 1;
+		} else if ((pm->node.flags | pm->old->node.flags) ==
+		    pm->old->node.flags) {
+		    /* private called twice on same parameter */
+		    Param tpm = pm;
+		    pm = pm->old;
+		    --locallevel;
+		    /* why have a union if we need this switch anyway? */
+		    switch (PM_TYPE(pm->node.flags)) {
+		    case PM_SCALAR:
+			pm->gsu.s->setfn(pm, tpm->u.str);
+			tpm->u.str = NULL;
+			break;
+		    case PM_INTEGER:
+			pm->gsu.i->setfn(pm, tpm->u.val);
+			break;
+		    case PM_EFLOAT:
+		    case PM_FFLOAT:
+			pm->gsu.f->setfn(pm, tpm->u.dval);
+			break;
+		    case PM_ARRAY:
+			pm->gsu.a->setfn(pm, tpm->u.arr);
+			tpm->u.arr = NULL;
+			break;
+		    case PM_HASHED:
+			pm->gsu.h->setfn(pm, tpm->u.hash);
+			tpm->u.hash = NULL;
+			break;
+		    }
+		    ++locallevel;
+		    if (!(tpm->node.flags & PM_UNSET))
+			pm->node.flags &= ~PM_UNSET;
+		} else {
+		    zerrnam("private",
+			    "can't change type of private param: %s",
+			    pm->node.nam);
+		    makeprivate_error = 1;
+		}
+	    } else {
+		zerrnam("private", "can't change scope of existing param: %s",
+			pm->node.nam);
+		makeprivate_error = 1;
+	    }
 	    return;
 	}
 	struct gsu_closure *gsu = zalloc(sizeof(struct gsu_closure));
diff --git a/Test/V10private.ztst b/Test/V10private.ztst
index b876f548d..d902cac56 100644
--- a/Test/V10private.ztst
+++ b/Test/V10private.ztst
@@ -384,6 +384,23 @@ F:Should we allow "public" namerefs to private parameters?
  }
 0:regression test for unset private
 
+ () {
+   private x=1
+   unset x
+   private x=2
+   print $x
+ }
+0:private may be called twice
+>2
+
+ () {
+   private x=1
+   private -a x
+   print $x
+ }
+1:private may not change parameter type
+?(anon):private:2: can't change type of private param: x
+
 %clean
 
   rm -r private.TMP