POSIX_BUILTINS and readonly parameters, again

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

This concerns the discussion that started in 34991, specifically the patch
34992 and the thread leading to 35004 where things dropped without further
resolution.

Part of the patch in 34992 is this:

@@ -874,10 +874,14 @@ createparam(char *name, int flags)
        DPUTS(oldpm && oldpm->level > locallevel,
              "BUG: old local parameter not deleted");
        if (oldpm && (oldpm->level == locallevel || !(flags & PM_LOCAL))) {
+           if (isset(POSIXBUILTINS) && (oldpm->node.flags & PM_READONLY)) {
+               zerr("read-only variable: %s", name);
+               return NULL;
+           }

Short question:

Why does that test POSIXBUILTINS as well as PM_READONLY ?

Long explanation:

The isset(POSIXBUILTINS) there seems to be covering a really strange edge
case.  If POSIXBUILTINS is not set, it's impossible to create a readonly
variable that is unset:

torch% () { readonly foo; print ${foo+foo is set}; unset foo }
foo is set
(anon): read-only variable: foo
torch% () { readonly foo; print ${foo+foo is set}; foo=oopsie }
foo is set
(anon): read-only variable: foo

[That first error is coming from unsetparam_pm() and the second from
assignsparam().  Neither is from the createparam() patch above, the
variable is set so createparam() never called.]

Conversely if POSIXBUILTINS is set, readonly variables are unset unless
assigned in the declaration [good thing we fixed array assignment there]:

torch% () { setopt localoptions posixbuiltins
 readonly foo; print ${foo-foo is NOT set}; unset foo }
foo is NOT set
(anon):1: read-only variable: foo
torch% () { setopt localoptions posixbuiltins
 readonly foo; print ${foo-foo is NOT set}; foo=oopsie }       
foo is NOT set
(anon):1: read-only variable: foo

[Note we still get the error for attempting to unset an already unset
parameter.  Probably wrong.  The first error is from unsetparam_pm()
and the second one is from createparam(), assignment is never tried.]

So the only reason for testing isset(POSIXBUILTINS) in createparam() is
the case where POSIXBUILTINS *changes* between time of declaration and
time of assignment:

torch% () { setopt localoptions posixbuiltins
 readonly foo; print ${foo-foo is NOT set};
 unsetopt posixbuiltins;             
 foo=oopsie; print ${foo-foo is NOT set} }
foo is NOT set
oopsie

This seems a little twitchy.  Having gone to the trouble of creating a
parameter that we know is both readonly and unset, we allow it to be
clobbered anyway?  Could this respect PM_READONLY unconditionally?

-- 
Barton E. Schaefer