Sanity of lastval ($?) in for/select loops

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Apr 19, 10:54am, Bart Schaefer wrote:
}
} ASIDE:
} 
} 	unset x
} 	for x in ${x:-$(exit 5)} y; do echo $?; done
} 
} outputs 5 in bash and 0 in zsh.  This suggests that perhaps execfor()
} should NOT be clobbering lastval.  (Same for "select".)

Data point: ksh93 agrees with bash here.

Furthermore, zsh execfor() and execselect() are inconsistent in their
handling of "lastval".  E.g., execfor() has this in the branch that
handles "for ((e1;e2;e3))" constructs:

        if (!errflag)
            matheval(str);
        if (errflag) {
            state->pc = end;
            return lastval = errflag;
        }

But later, in handling the loop body itself:

            if (errflag) {
                if (breaks)
                    breaks--;
                lastval = 1;
                break;
            }

So which should it be, lastval = 1 or lastval = errflag?  In practice I
don't think errflag is ever assigned anything other than 0 or 1 so it
probably isn't significant, just a bit confusing when reading the code.

However, execselect() then has:

    if (!args || empty(args)) {
	state->pc = end;
	return 1;
    }

Not "return lastval = 1" even though elsewhere it's careful to return
lastval.  Turns out the caller [execsimple() or execcmd()] always sets
lastval to whatever value is returned from execfor() or execselect()
or any of the other similar functions, so it's redundant to assign it
immediately before returning.

All of which presented as justification for this little patch:

diff --git a/Src/loop.c b/Src/loop.c
index dc8f232..2f639fd 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -73,7 +73,7 @@ execfor(Estate state, int do_exec)
 	    matheval(str);
 	if (errflag) {
 	    state->pc = end;
-	    return lastval = errflag;
+	    return 1;
 	}
 	cond = ecgetstr(state, EC_NODUP, &ctok);
 	advance = ecgetstr(state, EC_NODUP, &atok);
@@ -102,7 +102,7 @@ execfor(Estate state, int do_exec)
 		addlinknode(args, dupstring(*x));
 	}
     }
-    lastval = 0;
+    /* lastval = 0; */
     loops++;
     pushheap();
     cmdpush(CS_FOR);
@@ -241,7 +241,7 @@ execselect(Estate state, UNUSED(int do_exec))
 	return 1;
     }
     loops++;
-    lastval = 0;
+    /* lastval = 0; */
     pushheap();
     cmdpush(CS_SELECT);
     usezle = interact && SHTTY != -1 && isset(USEZLE);