Re: Error status of "repeat" (was Re: [PATCH] typeset: set $? on incidental error)

[Peter Stephenson <p.stephenson@samsung.com>]

On Tue, 26 Jan 2016 20:15:13 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Jan 26, 10:50pm, Daniel Shahaf wrote:
> }
> } What would you expect "repeat 0+++ (exit 42)" to set $? to?
>...
> diff --git a/Src/loop.c b/Src/loop.c
> index 4def9b6..61dad09 100644
> --- a/Src/loop.c
> +++ b/Src/loop.c
> @@ -493,7 +493,7 @@ execrepeat(Estate state, UNUSED(int do_exec))
>      tmp = ecgetstr(state, EC_DUPTOK, &htok);
>      if (htok)
>  	singsub(&tmp);
> -    count = atoi(tmp);
> +    count = mathevali(tmp);
>      pushheap();
>      cmdpush(CS_REPEAT);
>      loops++;

As it's documented that this is how it works, and it's hard to see how
it can break anything that already works since that will have to be an
integer at this point, that looks OK.

> -- that doesn't change the exit status!
> 
> torch% repeat 0+++ (exit 42)
> zsh: bad math expression: lvalue required
> torch% print $?
> 0

That message comes from a zerr() in the code.  It suggests repeat isn't
testing the error status between the code you've just added and
executing the loop --- as atoi() won't fail, just give potential
nonsense, this wasn't needed before.  Probably "if (errflag) return;"
before the pushheap() is correct.

The error comes from the expansion, not the loop itself which isn't
executed, so the status is going to be 1, and clearly repeating the loop
0 times if the +'s are stripped is going to give status 0.

The "if (htok) singsub();" is related to some not very obvious syntactic
sequence, but I remember I did manage to trigger the equivalent when
changing the "case" code last year.

pws