[PATCH] declarednull: felipec's approach

[Felipe Contreras <felipe.contreras@xxxxxxxxx>]

This patch merges Bart's approach with my approach, and should be
applied on top of the branch declarednull.

The main difference is that PM_DECLAREDNULL does not set PM_UNSET. So in
the two relevant places where PM_UNSET should be checked, PM_DELCAREDNULL
is checked too.

In my branch it's actually called PM_NULL because I think semantically
makes more sense. I just renamed it for this patch to minimize the diff.

A variable is initially declared with PM_NULL (same as PM_DECLAREDNULL),
then, once a value is a assigned, PM_NULL is removed.

That's it. Semantically it makes sense.

What doesn't make sense is to remove PM_DECLAREDNULL, because it's a
combination of PM_UNSET and PM_DECLARED. It makes sense to remove
PM_UNSET, but not to remove PM_DECLARED.

Why would this

  local var
  var=foobar

remove PM_DELCARED?

Additionally it's not clear why unset should clear the PM_DECLAREDNULL
flag. PM_UNSET is going to be immediately set anyway, and PM_DECLARED is
set only when there's no value (essentially PM_NULL), so it's kind of
returning to var='' (assuming there's a value).

I added a test that shows a discrepancy I found (${(t)var}) but there
could be many, may more. I only checked one instance of PM_UNSET.

Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
---
 Src/builtin.c      |  2 +-
 Src/params.c       | 11 ++++-------
 Src/subst.c        |  2 +-
 Src/zsh.h          |  3 +--
 Test/E03posix.ztst |  8 ++++++++
 5 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/Src/builtin.c b/Src/builtin.c
index 1e950f122..988233504 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2837,7 +2837,7 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 	    unqueue_signals();
 	    return 1;
 	} else if (pm) {
-	    if ((!(pm->node.flags & PM_UNSET) || pm->node.flags & PM_DECLARED)
+	    if (!(pm->node.flags & PM_UNSET)
 		&& (locallevel == pm->level || !(on & PM_LOCAL))) {
 		if (pm->node.flags & PM_TIED) {
 		    if (PM_TYPE(pm->node.flags) != PM_SCALAR) {
diff --git a/Src/params.c b/Src/params.c
index c09a3eccf..8912a7f65 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2093,8 +2093,7 @@ fetchvalue(Value v, char **pptr, int bracks, int flags)
 	if (sav)
 	    *s = sav;
 	*pptr = s;
-	if (!pm || ((pm->node.flags & PM_UNSET) &&
-		    !(pm->node.flags & PM_DECLARED)))
+	if (!pm || (pm->node.flags & PM_UNSET))
 	    return NULL;
 	if (v)
 	    memset(v, 0, sizeof(*v));
@@ -3625,7 +3624,6 @@ unsetparam_pm(Param pm, int altflag, int exp)
     else
 	altremove = NULL;
 
-    pm->node.flags &= ~PM_DECLARED;	/* like ksh, not like bash */
     if (!(pm->node.flags & PM_UNSET))
 	pm->gsu.s->unsetfn(pm, exp);
     if (pm->env)
@@ -5854,9 +5852,8 @@ printparamnode(HashNode hn, int printflags)
     Param peer = NULL;
 
     if (p->node.flags & PM_UNSET) {
-	if ((printflags & (PRINT_POSIX_READONLY|PRINT_POSIX_EXPORT) &&
-	     p->node.flags & (PM_READONLY|PM_EXPORTED)) ||
-	    (p->node.flags & PM_DECLAREDNULL) == PM_DECLAREDNULL) {
+	if (printflags & (PRINT_POSIX_READONLY|PRINT_POSIX_EXPORT) &&
+	    p->node.flags & (PM_READONLY|PM_EXPORTED)) {
 	    /*
 	     * Special POSIX rules: show the parameter as readonly/exported
 	     * even though it's unset, but with no value.
@@ -5971,7 +5968,7 @@ printparamnode(HashNode hn, int printflags)
 	}
     }
 
-    if ((printflags & PRINT_NAMEONLY) ||
+    if ((printflags & PRINT_NAMEONLY) || p->node.flags & PM_DECLAREDNULL ||
 	((p->node.flags & PM_HIDEVAL) && !(printflags & PRINT_INCLUDEVALUE)))
 	quotedzputs(p->node.nam, stdout);
     else {
diff --git a/Src/subst.c b/Src/subst.c
index 8f5bd355e..89d6abbba 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2532,7 +2532,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 			     (wantt ? -1 :
 			      ((unset(KSHARRAYS) || inbrace) ? 1 : -1)),
 			     scanflags)) ||
-	    (v->pm && (v->pm->node.flags & PM_UNSET)) ||
+	    (v->pm && ((v->pm->node.flags & PM_UNSET) || (v->pm->node.flags & PM_DECLAREDNULL))) ||
 	    (v->flags & VALFLAG_EMPTY))
 	    vunset = 1;
 
diff --git a/Src/zsh.h b/Src/zsh.h
index 6d7f517c6..97d3a142a 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1929,10 +1929,8 @@ struct tieddata {
 				   made read-only by the user               */
 #define PM_READONLY_SPECIAL (PM_SPECIAL|PM_READONLY|PM_RO_BY_DESIGN)
 #define PM_DONTIMPORT	(1<<22)	/* do not import this variable              */
-#define PM_DECLARED	(1<<22) /* explicitly named with typeset            */
 #define PM_RESTRICTED	(1<<23) /* cannot be changed in restricted mode     */
 #define PM_UNSET	(1<<24)	/* has null value                           */
-#define PM_DECLAREDNULL (PM_DECLARED|PM_UNSET)
 #define PM_REMOVABLE	(1<<25)	/* special can be removed from paramtab     */
 #define PM_AUTOLOAD	(1<<26) /* autoloaded from module                   */
 #define PM_NORESTORE	(1<<27)	/* do not restore value of local special    */
@@ -1942,6 +1940,7 @@ struct tieddata {
 				 */
 #define PM_HASHELEM     (1<<28) /* is a hash-element */
 #define PM_NAMEDDIR     (1<<29) /* has a corresponding nameddirtab entry    */
+#define PM_DECLAREDNULL	(1<<30)	/* declared but null                        */
 
 /* The option string corresponds to the first of the variables above */
 #define TYPESET_OPTSTR "aiEFALRZlurtxUhHTkz"
diff --git a/Test/E03posix.ztst b/Test/E03posix.ztst
index 5e6eddeba..b66782eea 100644
--- a/Test/E03posix.ztst
+++ b/Test/E03posix.ztst
@@ -103,3 +103,11 @@
 >arg1 arg2
 >noktarg1
 >0 0
+
+  f () {
+    local var
+    print ${(t)var}
+  }
+  f
+0:(t) returns correct type
+>scalar-local
-- 
2.30.0.rc1