Re: [PATCH] Optimization of getarrvalue()

["Jun T." <takimoto-j@xxxxxxxxxxxxxxxxx>]

After the following commit

commit a1633e09a761b9135a0a7084d2489d359a004e5a
Author: Peter Stephenson <pws@xxxxxxx>
Date:   Wed Nov 9 11:54:57 2016 +0000

    39886 based on 39877: Optimise arrdup to arrdup_max.

I'm having a problem with (some of) the completions which use
_sequence. For example, the following will produce bogus output:

zsh% ps -p <TAB>
zsh% mount -t <TAB>

The problem comes from the last line of _sequence:

"${(@)argv[1,minus-1]}" .... "${(@)argv[minus+1,-1]}"

In the case of 'ps -p <TAB>', argv=( _pids ) and minus=2.
We expect that, since it has a (@) flag, "${(@)argv[3,-1]}" is
entirely removed from the command line. This is indeed the case
before the commit:

zsh% a=( foo )
zsh% nargs () { print $#@ }
zsh% nargs "${a[3,-1]}"
1
zsh% nargs "${(@)a[3,-1]}"
0

But after the commit a1633e0, both output 1. This means an
empty string '' is passed to _pids and then down to compadd.
I still don't understand why compadd is confused by '' in its
arguments.

The patch below seems to make it work as before the commit
a1633e0; 'ps -p <TAB>' etc. works again. BUT:

The original (before a1633e0) behavior is already quite
confusing to me. For example,

zsh% nargs "${(@)a[i]}"

will output 0 only for i=0. On the other hand

zsh% nargs "${(@)a[i,i]}"

will output 0 for i=0 and 2.
If I replace
   arrlen_lt(s, v->start) by arrlen_le(s, v->start)
(which may look reasonable since the array s[] is 0-based)
then nargs "${(@)a[i,i]}" will output 0 only for i=0.
But then "make check" fails in two tests (D04parameter and
Y01completion).



diff --git a/Src/params.c b/Src/params.c
index 6f587a3..eee1cb8 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2292,10 +2292,11 @@ getarrvalue(Value v)
 	v->end += arrlen(s) + 1;
 
     /* Null if 1) array too short, 2) index still negative */
-    if (arrlen_lt(s, v->start) || v->start < 0) {
-	s = arrdup_max(nular, 1);
-    } else if (v->end <= v->start) {
+    if (v->end <= v->start) {
         s = arrdup_max(nular, 0);
+    }
+    else if (arrlen_lt(s, v->start) || v->start < 0) {
+	s = arrdup_max(nular, 1);
     } else {
         /* Copy to a point before the end of the source array:
          * arrdup_max will copy at most v->end - v->start elements,