Re: New typeset is a bit too verbose, and skips some assignments

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Fri, 26 Jun 2015 14:56:42 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> Thanks for the patch, but it still doesn't address my second point:

Ah, you're simply saying "typeset thing=()" doesn't explicitly assign an
empty array.  (Long series of examples make my eyes glaze over, here as
elsewhere.)

The issue is the same as before --- asg->value.array is empty for an
array, but actually the fact that asg->is_array is set means there must
have been a value.  So the answer is a more general fix, replacing the
previous one --- ASB_VALUEP() is true any time asg->is_array is true.
One subtlety where we explicitly set asg->value.array to NULL in the
expectation it would be ignored now needs a variable to do the job.  (Or
we could get rid of asg->is_array but this is safer.)

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index 3da1678..ac5a568 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -452,11 +452,13 @@ execbuiltin(LinkList args, LinkList assigns, Builtin bn)
 		    if (asg->is_array) {
 			LinkNode arrnode;
 			fprintf(xtrerr, "=(");
-			for (arrnode = firstnode(asg->value.array);
-			     arrnode;
-			     incnode(arrnode)) {
-			    fputc(' ', xtrerr);
-			    quotedzputs((char *)getdata(arrnode), xtrerr);
+			if (asg->value.array) {
+			    for (arrnode = firstnode(asg->value.array);
+				 arrnode;
+				 incnode(arrnode)) {
+				fputc(' ', xtrerr);
+				quotedzputs((char *)getdata(arrnode), xtrerr);
+			    }
 			}
 			fprintf(xtrerr, " )");
 		    } else if (asg->value.scalar) {
@@ -1975,7 +1977,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	       int on, int off, int roff, Asgment asg, Param altpm,
 	       Options ops, int joinchar)
 {
-    int usepm, tc, keeplocal = 0, newspecial = NS_NONE, readonly;
+    int usepm, tc, keeplocal = 0, newspecial = NS_NONE, readonly, dont_set = 0;
     char *subscript;
 
     /*
@@ -2131,8 +2133,9 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
      */
     if (usepm) {
 	if (asg->is_array ?
-	    (asg->value.array && !(PM_TYPE(pm->node.flags) & (PM_ARRAY|PM_HASHED))) :
-	    (asg->value.scalar && (PM_TYPE(pm->node.flags & (PM_ARRAY|PM_HASHED))))) {
+	    !(PM_TYPE(pm->node.flags) & (PM_ARRAY|PM_HASHED)) :
+	    (asg->value.scalar && (PM_TYPE(pm->node.flags &
+					   (PM_ARRAY|PM_HASHED))))) {
 	    zerrnam(cname, "%s: inconsistent type for assignment", pname);
 	    return NULL;
 	}
@@ -2141,8 +2144,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	    if (OPT_ISSET(ops,'p'))
 		paramtab->printnode(&pm->node, PRINT_TYPESET);
 	    else if (!OPT_ISSET(ops,'g') &&
-		     (unset(TYPESETSILENT) || OPT_ISSET(ops,'m'))
-		     && !asg->is_array)
+		     (unset(TYPESETSILENT) || OPT_ISSET(ops,'m')))
 		paramtab->printnode(&pm->node, PRINT_INCLUDEVALUE);
 	    return pm;
 	}
@@ -2199,9 +2201,10 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	    DPUTS(ASG_ARRAYP(asg), "BUG: typeset got array value where scalar expected");
 	    if (asg->value.scalar && !(pm = setsparam(pname, ztrdup(asg->value.scalar))))
 		return NULL;
-	} else if (asg->value.array) {
-	    DPUTS(!ASG_ARRAYP(asg), "BUG: typeset got scalar value where array expected");
-	    if (!(pm = setaparam(pname, zlinklist2array(asg->value.array))))
+	} else if (asg->is_array) {
+	    if (!(pm = setaparam(pname, asg->value.array ?
+				 zlinklist2array(asg->value.array) :
+				 mkarray(NULL))))
 		return NULL;
 	}
 	pm->node.flags |= (on & PM_READONLY);
@@ -2211,7 +2214,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
     }
 
     if (asg->is_array ?
-	(asg->value.array && !(on & (PM_ARRAY|PM_HASHED))) :
+	!(on & (PM_ARRAY|PM_HASHED)) :
 	(asg->value.scalar && (on & (PM_ARRAY|PM_HASHED)))) {
 	zerrnam(cname, "%s: inconsistent type for assignment", pname);
 	return NULL;
@@ -2343,20 +2346,21 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	     */
 	    if (!(pm = setsparam(pname, ztrdup(asg->value.scalar ? asg->value.scalar : ""))))
 		return NULL;
-	    asg->value.scalar = NULL;
+	    dont_set = 1;
 	    asg->is_array = 0;
 	    keeplocal = 0;
 	    on = pm->node.flags;
 	} else if (PM_TYPE(on) == PM_ARRAY && ASG_ARRAYP(asg)) {
-	    if (!(pm = setaparam(pname, asg->value.array ? zlinklist2array(asg->value.array) :
+	    if (!(pm = setaparam(pname, asg->value.array ?
+				 zlinklist2array(asg->value.array) :
 				 mkarray(NULL))))
 		return NULL;
-	    asg->value.array = NULL;
+	    dont_set = 1;
 	    keeplocal = 0;
 	    on = pm->node.flags;
 	} else {
 	    zerrnam(cname,
-		    "%s: inconsistent array element or slice assignment", pname);
+		    "%s: i1;nconsistent array element or slice assignment", pname);
 	    return NULL;
 	}
     }
@@ -2422,11 +2426,13 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	pm->level = keeplocal;
     else if (on & PM_LOCAL)
 	pm->level = locallevel;
-    if (ASG_VALUEP(asg)) {
+    if (ASG_VALUEP(asg) && !dont_set) {
 	Param ipm = pm;
 	if (pm->node.flags & (PM_ARRAY|PM_HASHED)) {
 	    DPUTS(!ASG_ARRAYP(asg), "BUG: inconsistent scalar value for array");
-	    if (!(pm=setaparam(pname, zlinklist2array(asg->value.array))))
+	    if (!(pm=setaparam(pname, asg->value.array ?
+			       zlinklist2array(asg->value.array) :
+			       mkarray(NULL))))
 		return NULL;
 	} else {
 	    DPUTS(ASG_ARRAYP(asg), "BUG: inconsistent array value for scalar");
diff --git a/Src/zsh.h b/Src/zsh.h
index ee06094..ce9b979 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1162,13 +1162,11 @@ struct asgment {
 
 /*
  * Assignment has value?
- * We need to arrange for each of the values
- * to be the same type or the compiler will
- * get fed up.
+ * If the assignment is an arrray, then it certainly has a value --- we
+ * can only tell if there's an expicit assignment.
  */
 
-#define ASG_VALUEP(asg) (ASG_ARRAYP(asg) ?			\
-			 ((asg)->value.array != (LinkList)0) :	\
+#define ASG_VALUEP(asg) (ASG_ARRAYP(asg) ||			\
 			 ((asg)->value.scalar != (char *)0))
 
 /* node in command path hash table (cmdnamtab) */
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 5d69e5d..4c033cc 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -691,3 +691,19 @@
 >4 one umm er five
 >2 one five
 >4 nothing to see here
+
+  array=(no really nothing here)
+  fn() {
+    typeset array=() array[2]=two array[4]=four
+    typeset -p array
+    typeset array=() array[3]=three array[1]=one
+    typeset -p array
+  }
+  fn
+  print $array
+0:setting empty array in typeset
+>typeset -a array
+>array=('' two '' four)
+>typeset -a array
+>array=(one '' three)
+>no really nothing here