Re: Incorrect evaluation of ~ test in ternary conditional

[Felix Uhl <felix.uhl@xxxxxxxxxx>]

On 16.12.2017 23:01, Bart Schaefer wrote:
> On Fri, Dec 15, 2017 at 5:55 AM, Felix Uhl <felix.uhl@xxxxxxxxxx> wrote:
>> Hi everybody!
>>
>> Let there be a directory ~/work. Using the ~ test character in a
>> conditional ternary prompt will return incorrect results when the
>> argument is 2 as shown below:
>>
>> $ cd && print -P "%(2~:true:false)"
>> false
>> $ cd work/.. && print -P "%(2~:true:false)"
>> true
> I actually get randomly incorrect results in the FIRST of your examples:
>
> repeat 5 do cd && print -P "%~ %(2~:true:false)" ; done
> ~ true
> ~ false
> ~ true
> ~ false
> ~ false
>
> The second case is consistent but is always wrong:
>
> repeat 5 do cd work/.. && print -P "%~ %(2~:true:false)" ; done
> ~ true
> ~ true
> ~ true
> ~ true
> ~ true
>
> Both of the foregoing happen in each of 5.0.2 and the latest
> development version.  I don't have 5.0.5 handy.
>
>

Huh, interesting. Even at 5000 repetitions, I don't get a single wrong 
result on the first example (takes about 2 minutes to run though):

$ repeat 5000 do cd && print -nP "%(2~:true:)" ; done
$

I wonder where that discrepancy comes from.

> This seems to be the fix:
>
> diff --git a/Src/prompt.c b/Src/prompt.c
> index c478e69..63e8093 100644
> --- a/Src/prompt.c
> +++ b/Src/prompt.c
> @@ -315,7 +315,7 @@ putpromptchar(int doprint, int endchar, unsigned
> int *txtchangep)
>                  case '/':
>                  case 'C':
>                      /* `/' gives 0, `/any' gives 1, etc. */
> -                   if (*ss++ == '/' && *ss)
> +                   if (*ss && *ss++ == '/' && *ss)
>                          arg--;
>                      for (; *ss; ss++)
>                          if (*ss == '/')
>
> I'm not sure whether (*ss == '/' && *++ss) would be equivalent, i.e.,
> I don't know why the original formulation skips over the first
> character whether or not it is a '/'.  Possibly to skip a leading '~'?

The original implementation doesn't skip the first character, does it?
C operator precedence will treat *ss++ like *(ss++), so if ss is (for 
the sake of demonstration) 10, it is incremented to 11, but the postfix 
operator returns the old value of 10, which is then dereferenced by *. 
So the whole condition checks whether the first character is '/' and 
whether the second character is not 0.

I'd probably write that as (*ss == '/' && *(++ss)), seems much clearer 
to me.

I fail to understand why your fix should work, the expressions (*ss && 
*ss++ == '/') and (*ss++ == '/') on their own are logically equivalent 
to (*ss == '/') and have equivalent side-effects as well.
Did you actually test it?