[PATCH] (take 2) Local specials and Re: Regression of typeset output with "private"

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Sun, Feb 18, 2024 at 11:35 AM Bart Schaefer
<schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> How about this?  It skips PM_RO_BY_DESIGN for any parameter not
> declared in the same scope where "typeset -p" is being run.  This
> means that in addition to displaying parameters declared private,
> it'll show cases where "local +h" has been used to shadow a special.

The more I think through this, the more I think it's the right thing
to do.  A couple of related observations:
1) It isn't necessary to use +h unless removing the -h attribute, the
default is to retain specialness even when local.  I usually include
it just for clarity, so in case we want to add it later I've left a
comment in the patch.
2) On the other hand, it is necessary to use -h for hiding, and
typeset -p did not preserve that flag on output before.

Consequently I've revised the patch to add the -h option for
parameters that have it.  This is also consistent with the output of
${(t)var} which shows the substring "hide".  The side-effect of this
is that private parameters are shown as having the -h property, which
is a bit closer to correct.

> [...] we should probably mention
> somewhere that "typeset -p" no longer displays values for the special
> parameters $!, $#, $-, $*, $@, $$, $?, $ARGC, etc.

Doc change added for this.  Also disabled "private -p" (it had already
been removed from the documentation) and update the doc to reflect
this.

> This is actually a change in 5.9 vs. 5.8 that was not
> included in the NEWS file.

I did not edit NEWS, yet.

This replaces workers/52557.

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 7a8654f27..784089594 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -2136,6 +2136,14 @@ tt(-p) may be followed by an optional integer argument.  Currently
 only the value tt(1) is supported.  In this case arrays and associative
 arrays are printed with newlines between indented elements for
 readability.
+
+The names and values of readonly special parameters
+(most of the parameters marked `<S>' in
+ifzman(zmanref(zshparam))ifnzman(noderef(Parameters Set By The Shell)),
+except those documented as settable)
+are not printed with `tt(-)tt(p)' because to execute those typeset commands
+would cause errors.  However, these parameters are printed when they
+have been made local to the scope where `tt(typeset -p)' is run.
 )
 item(tt(-T) [ var(scalar)[tt(=)var(value)] var(array)[tt(=LPAR())var(value) ...tt(RPAR())] [ var(sep) ] ])(
 This flag has a different meaning when used with tt(-f); see below.
diff --git a/Doc/Zsh/mod_private.yo b/Doc/Zsh/mod_private.yo
index 08ac4cafe..24c099f38 100644
--- a/Doc/Zsh/mod_private.yo
+++ b/Doc/Zsh/mod_private.yo
@@ -16,9 +16,10 @@ The tt(private) builtin accepts all the same options and arguments as tt(local)
 (ifzman(zmanref(zshbuiltins))ifnzman(noderef(Shell Builtin Commands))) except
 for the `tt(-)tt(T)' option.  Tied parameters may not be made private.
 
-The `tt(-)tt(p)' option is presently a no-op because the state of
-private parameters cannot reliably be reloaded.  This also applies
-to printing private parameters with `tt(typeset -p)'.
+The `tt(-)tt(p)' option is presently disabled because the state of
+private parameters cannot reliably be reloaded.  When `tt(typeset -)tt(p)'
+outputs a private parameter, it is treated as a local with the
+`tt(-)tt(h)' (hide) option enabled.
 
 If used at the top level (outside a function scope), tt(private) creates a
 normal parameter in the same manner as tt(declare) or tt(typeset).  A
diff --git a/Src/Modules/param_private.c b/Src/Modules/param_private.c
index 5003d4627..044617190 100644
--- a/Src/Modules/param_private.c
+++ b/Src/Modules/param_private.c
@@ -646,7 +646,7 @@ printprivatenode(HashNode hn, int printflags)
 
 static struct builtin bintab[] = {
     /* Copied from BUILTIN("local"), "P" added */
-    BUILTIN("private", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_private, 0, -1, 0, "AE:%F:%HL:%PR:%TUZ:%ahi:%lnmprtux", "P")
+    BUILTIN("private", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_private, 0, -1, 0, "AE:%F:%HL:%PR:%TUZ:%ahi:%lnmrtux", "P")
 };
 
 static struct features module_features = {
diff --git a/Src/params.c b/Src/params.c
index fce3af940..b329d2079 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -5896,6 +5896,7 @@ static const struct paramtypes pmtypes[] = {
     { PM_ARRAY, "array", 'a', 0},
     { PM_HASHED, "association", 'A', 0},
     { 0, "local", 0, PMTF_TEST_LEVEL},
+    { PM_HIDE, "hide", 'h', 0 },
     { PM_LEFT, "left justified", 'L', PMTF_USE_WIDTH},
     { PM_RIGHT_B, "right justified", 'R', PMTF_USE_WIDTH},
     { PM_RIGHT_Z, "zero filled", 'Z', PMTF_USE_WIDTH},
@@ -6025,13 +6026,21 @@ printparamnode(HashNode hn, int printflags)
 	printflags |= PRINT_NAMEONLY;
 
     if (printflags & (PRINT_TYPESET|PRINT_POSIX_READONLY|PRINT_POSIX_EXPORT)) {
-	if (p->node.flags & (PM_RO_BY_DESIGN|PM_AUTOLOAD)) {
+	if (p->node.flags & PM_AUTOLOAD) {
 	    /*
 	     * It's not possible to restore the state of
 	     * these, so don't output.
 	     */
 	    return;
 	}
+	if (p->node.flags & PM_RO_BY_DESIGN) {
+	    /*
+	     * Compromise: cannot be restored out of context,
+	     * but show anyway if printed in scope of declaration
+	     */
+	    if (p->level != locallevel || p->level == 0)
+		return;
+	}
 	/*
 	 * The zsh variants of export -p/readonly -p also report other
 	 * flags to indicate other attributes or scope. The POSIX variants
@@ -6064,8 +6073,19 @@ printparamnode(HashNode hn, int printflags)
 	for (pmptr = pmtypes, i = 0; i < PMTYPES_SIZE; i++, pmptr++) {
 	    int doprint = 0;
 	    if (pmptr->flags & PMTF_TEST_LEVEL) {
-		if (p->level)
+		if (p->level) {
+		    /*
+		    if ((p->node.flags & PM_SPECIAL) &&
+			(p->node.flags & PM_LOCAL) &&
+			!(p->node.flags & PM_HIDE)) {
+			if (doneminus)
+			    putchar(' ');
+			printf("+h ");
+			doneminus = 0;
+		    }
+		    */
 		    doprint = 1;
+		}
 	    } else if ((pmptr->binflag != PM_EXPORTED || p->level ||
 			(p->node.flags & (PM_LOCAL|PM_ARRAY|PM_HASHED))) &&
 		       (p->node.flags & pmptr->binflag))
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 8b3988151..d90f17d13 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -959,6 +959,20 @@
 >  [three]=''
 >)
 
+ () {
+  local -h status
+  typeset -p status
+ }
+0:parameter hiding preserved by "typeset -p"
+>typeset -h status=''
+
+ () {
+  local status
+  typeset -p status
+ }
+0:read-only special params are output when localized
+>typeset -i10 -r status=0
+
  (export PATH MANPATH
  path=(/bin)
  MANPATH=/
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index ebb70dd92..ff48e2289 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -464,7 +464,7 @@ F:unexpected side-effects of previous tests
  }
 0:up-reference part 3, hidden global
 >outside
->typeset var
+>typeset -h var
 
  () {
    typeset notdef
@@ -541,7 +541,7 @@ F:Same test, should part 5 output look like this?
  fi
 0:up-reference part 3, autoloading with hidden special
 >nameref-local-nameref-local
->typeset parameters
+>typeset -h parameters
 
  if [[ $options[typesettounset] != on ]]; then
    ZTST_skip='Ignoring zmodload bug that resets TYPESET_TO_UNSET'
diff --git a/Test/V10private.ztst b/Test/V10private.ztst
index 9eeda0f47..4140d4e96 100644
--- a/Test/V10private.ztst
+++ b/Test/V10private.ztst
@@ -28,7 +28,7 @@
  print $scalar_test
 0:basic scope hiding
 >toplevel
->local scalar_test
+>local hide scalar_test
 >0
 >toplevel
 
@@ -54,7 +54,7 @@
  print $+unset_test
 0:variable defined only in scope
 >0
->local unset_test
+>local hide unset_test
 >setme
 >0
 
@@ -70,7 +70,7 @@
  }
  print $array_test
 0:nested scope with different type, correctly restored
->local array_test
+>local hide array_test
 >in function
 >top level
 
@@ -113,7 +113,7 @@
  typeset -a hash_test=(top level)
  typeset -p hash_test
  inner () {
-  private -p hash_test
+  typeset -p hash_test
   print ${(t)hash_test} ${(kv)hash_test}
  }
  outer () {
@@ -328,6 +328,7 @@ F:future revision will create a global with this assignment
 F:See K01nameref.ztst up-reference part 5
 F:Here ptr1 finds private ptr2 by scope mismatch
 >typeset -n ptr1=ptr2
+>typeset -hn ptr2
 *?*read-only variable: ptr2
 
  () {
@@ -348,10 +349,12 @@ F:See K01nameref.ztst up-reference part 5
 F:Here ptr1 finds private ptr2 by scope mismatch
 F:Assignment silently fails, is that correct?
 >typeset -n ptr1=ptr2
+>typeset -hn ptr2=''
 >ptr1=ptr2
 >ptr1=
 >ptr2=
 >typeset -n ptr1=ptr2
+>typeset -hn ptr2=''
 *?*no such variable: ptr2
 
  typeset ptr2
@@ -372,11 +375,13 @@ F:Assignment silently fails, is that correct?
 F:See K01typeset.ztst up-reference part 5
 F:Here ptr1 points to global ptr2 so assignment succeeds
 >typeset -n ptr1=ptr2
+>typeset -hn ptr2
 >ptr1=ptr2
 >ptr2=val
 >ptr1=val
 >ptr2=val
 >typeset -n ptr1=ptr2
+>typeset -hn ptr2
 >typeset ptr2=val
 
  () {