[PATCH] namerefs and specials and scoping (oh my)

[Bart Schaefer <schaefer@brasslantern.com>]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

Another obscure one, but with possibly surprising side-effects.  In
createparam(), we first call gethashnode2() to read the global
paramtab directly and then decide whether a new parameter is being
created or an existing (but unset) one is being restored.

However, for readonly special parameters, calling the paramtab API
getnode2 might be necessary to find the right scope for the new
parameter ... or more precisely, to find out if there is either
already a parameter in that scope that should not be restored, or a
named reference whose referent should be neither created nor restored.

This mostly affects parameters being assigned by addvars(), that is,
assignment expressions with no declaration.  If "typeset" is being
used, it only matters when the -g option is passed, because otherwise
a local is being created.

The most consistent place to add test for this is with private namerefs.

[-- Attachment #2: scoped-nameref.txt --]
[-- Type: text/plain, Size: 2518 bytes --]

diff --git a/Src/params.c b/Src/params.c
index b329d2079..225acb8a1 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1004,8 +1004,29 @@ createparam(char *name, int flags)
 			 gethashnode2(paramtab, name) :
 			 paramtab->getnode(paramtab, name));
 
-	if (oldpm && (oldpm->node.flags & PM_NAMEREF) &&
-	    !(flags & PM_NAMEREF) && (oldpm = upscope(oldpm, oldpm->base))) {
+	if (oldpm && (oldpm->node.flags & PM_RO_BY_DESIGN)) {
+	    if (!(flags & PM_LOCAL)) {
+		/* Must call the API for namerefs and specials to work */
+		pm = (Param) paramtab->getnode2(paramtab, oldpm->node.nam);
+		if (!pm || ((pm->node.flags & PM_NAMEREF) &&
+			    pm->level != locallevel)) {
+		    zerr("%s: can't modify read-only parameter", name);
+		    return NULL;
+		}
+	    }
+	    /**
+	     * Implementation note: In the case of a readonly nameref,
+	     * the right thing might be to insert a new global into
+	     * the paramtab and point the local pm->old at it, rather
+	     * than error.  That is why gethashnode2() is called
+	     * first, to avoid skipping up the stack prematurely.
+	     **/
+	}
+
+	if (oldpm && !(flags & PM_NAMEREF) &&
+	    (!(oldpm->node.flags & PM_RO_BY_DESIGN) || !(flags & PM_LOCAL)) &&
+	    (oldpm->node.flags & PM_NAMEREF) &&
+	    (oldpm = upscope(oldpm, oldpm->base))) {
 	    Param lastpm;
 	    struct asgment stop;
 	    stop.flags = PM_NAMEREF | (flags & PM_LOCAL);
diff --git a/Test/V10private.ztst b/Test/V10private.ztst
index 4140d4e96..efa346002 100644
--- a/Test/V10private.ztst
+++ b/Test/V10private.ztst
@@ -311,6 +311,47 @@ F:future revision will create a global with this assignment
 >TOP
 >UP:
 
+ () {
+   typeset -a ary
+   local -P -n ref=ary
+   {
+    (){
+     ref=XX	# Should be an error
+     typeset -p ary ref
+    }
+   } always {
+    TRY_BLOCK_ERROR=0
+    typeset -p ary ref
+   }
+ }
+ typeset -p ary
+1:assignment to private nameref in wrong scope, part 1
+>typeset -a ary
+>typeset -hn ref=ary
+*?*ref: can't modify read-only parameter
+*?*no such variable: ary
+
+ () {
+   typeset -a ary
+   local -P -n ref=ary
+   {
+    (){
+     typeset ref=XX	# Should create a local
+     typeset -p ary ref
+    }
+   } always {
+    TRY_BLOCK_ERROR=0
+    typeset -p ary ref
+   }
+ }
+ typeset -p ary
+1:assignment to private nameref in wrong scope, part 2
+>typeset -g -a ary
+>typeset ref=XX
+>typeset -a ary
+>typeset -hn ref=ary
+*?*no such variable: ary
+
  () {
    typeset -n ptr1=ptr2
    private -n ptr2	# TYPESET_TO_UNSET makes this not a "placeholder"