51945: assorted documentation improvements, bug fixes, and new test

1) Document the behavior of "typeset -n existing_var" (via Jun T. comment)
2) Prohibit "typeset -nm pattern" because, well, it's insane.  Add test.
3) Improve doc for ${(!)ref} including ${{t!)ref} (Jun T.)
4) Fix doc for how-to unset of a named ref (Jun T.)
5) Allow "typeset +r -n ref" and "typeset +r +n ref" (Jun T.)
6) Fix "typeset -r -n ref=param" to create readonly references
7) Avoid accidental removal of PM_UNSET flag (Jun T.) and update test
8) Fix "typeset -gn ref=value" and add a test for it
9) Add tests for read-only reference behavior
10) Fix infinite recursion when resolving scope of an unset local
named reference, add test.

2 files changed, 42 insertions, 13 deletions

diff --git a/Src/builtin.c b/Src/builtin.c
index 1568cf44c..31af66c7c 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2248,10 +2248,14 @@ typeset_single(char *cname, char *pname, Param pm, int func,
 	    zerrnam(cname, "%s: restricted", pname);
 	    return pm;
 	}
-	if ((pm->node.flags & PM_READONLY) &&
-	    (pm->node.flags & PM_NAMEREF & off)) {
-	    zerrnam(cname, "%s: read-only reference", pname);
-	    return pm;
+	if ((pm->node.flags & PM_READONLY) && !(off & PM_READONLY) &&
+	    /* It seems as though these checks should not be specific to
+	     * PM_NAMEREF, but changing that changes historic behavior */
+	    ((on & PM_NAMEREF) != (pm->node.flags & PM_NAMEREF) ||
+	     (asg && (pm->node.flags & PM_NAMEREF)))) {
+	    zerrnam(cname, "%s: read-only %s", pname,
+		    (pm->node.flags & PM_NAMEREF) ? "reference" : "variable");
+	    return NULL;
 	}
 	if ((on & PM_UNIQUE) && !(pm->node.flags & PM_READONLY & ~off)) {
 	    Param apm;
@@ -2693,7 +2697,7 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 	    off |= bit;
     }
     if (OPT_MINUS(ops,'n')) {
-	if ((on & ~PM_READONLY)|off) {
+	if ((on|off) & ~PM_READONLY) {
 	    zwarnnam(name, "no other attributes allowed with -n");
 	    return 1;
 	}
@@ -3021,6 +3025,13 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
     /* With the -m option, treat arguments as glob patterns */
     if (OPT_ISSET(ops,'m')) {
 	if (!OPT_ISSET(ops,'p')) {
+	    if (on & PM_NAMEREF) {
+		/* It's generally unwise to mass-change the types of
+		 * parameters, but for namerefs it would be fatal */
+		unqueue_signals();
+		zerrnam(name, "invalid reference");
+		return 1;
+	    }
 	    if (!(on|roff))
 		printflags |= PRINT_TYPE;
 	    if (!on)
@@ -3104,13 +3115,25 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 	    }
 	    if (hn) {
 		/* namerefs always start over fresh */
-		if (((Param)hn)->level >= locallevel) {
+		if (((Param)hn)->level >= locallevel ||
+		    (!(on & PM_LOCAL) && ((Param)hn)->level < locallevel)) {
 		    Param oldpm = (Param)hn;
-		    if (!asg->value.scalar && oldpm->u.str)
+		    if (!asg->value.scalar &&
+			PM_TYPE(oldpm->node.flags) == PM_SCALAR &&
+			oldpm->u.str)
 			asg->value.scalar = dupstring(oldpm->u.str);
-		    unsetparam_pm((Param)hn, 0, 1);
+		    /* Defer read-only error to typeset_single() */
+		    if (!(hn->flags & PM_READONLY))
+			unsetparam_pm(oldpm, 0, 1);
 		}
-		hn = NULL;
+		/* Passing a NULL pm to typeset_single() makes the
+		 * nameref read-only before assignment, which breaks
+		 *   typeset -rn ref=var
+		 * so this is special-cased to permit that action
+		 * like assign-at-create for other parameter types.
+		 */
+		if (!(hn->flags & PM_READONLY))
+		    hn = NULL;
 	    }
 	}
 
diff --git a/Src/params.c b/Src/params.c
index f5750a4b4..5841308d7 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -546,7 +546,7 @@ getparamnode(HashTable ht, const char *nam)
 	}
     }
 
-    if (hn && ht == realparamtab)
+    if (hn && ht == realparamtab && !(hn->flags & PM_UNSET))
 	hn = resolve_nameref((Param)hn, NULL);
     return hn;
 }
@@ -3729,7 +3729,9 @@ unsetparam_pm(Param pm, int altflag, int exp)
     char *altremove;
 
     if ((pm->node.flags & PM_READONLY) && pm->level <= locallevel) {
-	zerr("read-only variable: %s", pm->node.nam);
+	zerr("read-only %s: %s",
+	     (pm->node.flags & PM_NAMEREF) ? "reference" : "variable",
+	     pm->node.nam);
 	return 1;
     }
     if ((pm->node.flags & PM_RESTRICTED) && isset(RESTRICTED)) {
@@ -6182,8 +6184,12 @@ resolve_nameref(Param pm, const Asgment stop)
 		seek = refname;
 	}
     }
-    else if (pm && !(stop && (stop->flags & PM_NAMEREF)))
-	return (HashNode)pm;
+    else if (pm) {
+	if (!(stop && (stop->flags & PM_NAMEREF)))
+	    return (HashNode)pm;
+	if (!(pm->node.flags & PM_NAMEREF))
+	    return (pm->level < locallevel ? NULL : (HashNode)pm);
+    }
     if (seek) {
 	queue_signals();
 	/* pm->width is the offset of any subscript */