Re: For loops and closures

[RLake@...]

>>       if (ttisnil(ra)) {  /* break loop? */
>>         setobjs2s(ra-1, ra);
>>         pc++;  /* skip jump (break loop) */
>>       }

>I believe this 2nd line should be in the "else" part, right?

Quite right. Sigh.

> I agree that new bindings may be useful sometimes, while keeping the
> same bindings have no particular use.

Great.

> The reason for keeping the same bindings is that the control variable is
> a kind of accumulator, which is updated as "var = f(var)" for each loop.
> To me it sounds a little weird to have an accumulator construction that
> creates new bindings as it goes.

I guess that depends on how you look at it. I think of it as an iterator,
not an accumulator, so it seems to me to be a new object each time. Of
course, I am used to Scheme, which uses precisely the "new binding"
definition.

> So, altough the code may be simpler
> for new bindings, it seems that the explanation (in terms of equivalent
> code) is more complex.

do
  local _f, _s, _iter = explist
  while true do
    local var_1, var_2, ... , var_n
    var_1, ..., var_n = _f(_s, _iter)
    if var_1 == nil then break end
    _iter = var_1
    block
  end
end

Well, it is one line longer. :) But it clearly demonstrates which are
internal values and which are loop values; also, it allows an answer to the
question "what happens if I assign to the iteration variables?" (Answer:
their value changes within the loop, but does not affect the iteration.)

> Other questions concern the numeric loop: should
> we change it too? (In that case the implementation would be slower and
> more complex.) Does it make sense to have each loop with a different
> binding?

I would say that it should be changed, too. The naive implementation is
exactly one line longer:

case OP_FORLOOP: {
  lua_Number step, idx, limit;
  const TObject *plimit = ra+1;
  const TObject *pstep = ra+2;
  if (!ttisnumber(ra))
    luaG_runerror(L, "`for' initial value must be a number");
  if (!tonumber(plimit, ra+1))
    luaG_runerror(L, "`for' limit must be a number");
  if (!tonumber(pstep, ra+2))
    luaG_runerror(L, "`for' step must be a number");
  step = nvalue(pstep);
  idx = nvalue(ra) + step;  /* increment index */
  limit = nvalue(plimit);
  if (step > 0 ? idx <= limit : idx >= limit) {
    dojump(pc, GETARG_sBx(i));  /* jump back */
    chgnvalue(ra, idx);  /* update index */
    setobjs2s(ra+3, ra); /* This line added */
  }
  break;
}

However, if the internal variables are hidden from the Lua program, there
is no need to do the checks on each iteration; it could be done in an
OP_FORPREP opcode. (In fact, that is true regardless except for the
iteration variable.) Then we have:

case OP_FORLOOP: {
  lua_Number step, idx, limit;
  step = nvalue(ra+2);
  idx = nvalue(ra) + step;  /* increment index */
  limit = nvalue(ra+1);
  if (step > 0 ? idx <= limit : idx >= limit) {
    dojump(pc, GETARG_sBx(i));  /* jump back */
    chgnvalue(ra, idx);  /* update index */
    chgnvalue(ra+3, idx); /* In this case, we know it is a number */
  }
  break;
}

case OP_FORPREP: {
  const TObject *plimit = ra+1;
  const TObject *pstep = ra+2;
  if (!ttisnumber(ra))
    luaG_runerror(L, "`for' initial value must be a number");
  if (!tonumber(plimit, ra+1))
    luaG_runerror(L, "`for' limit must be a number");
  if (!tonumber(pstep, ra+2))
    luaG_runerror(L, "`for' step must be a number");
  /* protect against the case where step is NaN */
  if ((step == step) && (step > 0 ? idx <= limit : idx >= limit))
    setobjs2s(ra+3, ra);
  else
    dojump(pc, GETARG_sBx(i));  /* skip the loop altogether */
  break;
}

The OP_FORPREP replaces the OP_SUB / OP_JUMP combination in the current
code, so it is one fewer VM op generated and two fewer ops executed. The
total code size of the two opcodes is only slightly larger than the size of
the current one.

By the way, I'm not convinced that the behaviour is "correct" in either the
current code or the above code in the case where step == 0. Logically, that
should be an endless loop regardless of idx and limit. Perhaps the test
ought to be:
   (step == 0) || (step > 0 ? idx <= limit : idx >= limit)

Actually, OP_TFORPREP could also do the first generation instead of doing a
jump to the OP_TFORLOOP, similarly saving one VM opcode execution at the
expense of a few more lines in the VM.