Re: [PATCH] Optimization of getarrvalue()

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

On 2016/11/16, at 4:57, Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:

> On Mon, 14 Nov 2016 21:32:19 +0900
> "Jun T." <takimoto-j@xxxxxxxxxxxxxxxxx> wrote:
>> 
>> 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.
> 
> 0 is an invalid subscript, which is probably the difference in the first
> case between it and other values.

The document says (15.2.1)

   If  the  KSH_ARRAYS  option  is not set, then by default accesses to an
   array element with a subscript that evaluates to zero return  an  empty
   string,

so someone may expect that "${(@)a[0]}" is also replaced by an empty
string "" rather than removed from the command line. But I'm not sure.
And I fear changing this behavior might break some existing scripts.

> setopt rcexpandparam
> local -A hash=(X x)                       
> print LOST key=$hash[(I)y] val=$hash[(R)Y]
> 
> outputs LOST with the arguments removed.  With your change you get an
> array wth an empty element and that doesn't happen.

I haven't yet understood how associative arrays are handled in C code,
but how about the following patch? With this patch all the test passes,
and "${(@)a[i,i]}" becomes an empty array (and removed from the command
line) only if i=0. I think no (reasonable) scripts are relying on the
current behavior that "${(@)a[i,i]}" is removed if i==($#a+1).


diff --git a/Src/params.c b/Src/params.c
index 3c8658c..c501e5d 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2291,11 +2291,12 @@ getarrvalue(Value v)
     if (v->end < 0)
 	v->end += arrlen(s) + 1;
 
-    /* Null if 1) array too short, 2) index still negative */
-    if (v->end <= v->start) {
+    if (!*s || v->end <= v->start) {
+	/* Empty array if 1) s[] is empty or 2) inconsistent indexes */
 	s = arrdup_max(nular, 0);
     }
-    else if (arrlen_lt(s, v->start) || v->start < 0) {
+    else if (arrlen_le(s, v->start) || v->start < 0) {
+	/* Null if 1) array too short, 2) index still negative */
 	s = arrdup_max(nular, 1);
     } else {
         /* Copy to a point before the end of the source array: