Re: Assignment statement does not evaluate all its expressions first

[Xmilia Hermit <xmilia.hermit@...>]

> This is a bug. Thanks for the report. The question now is whether we
> fix the implementation or the manual.

Either would be fine by me. For the fix in the implementation I can 
propose the following patch. There are some things that require cleanup 
though:
1) I am not certain if reg < fs->nactvar is valid to check if reg is a 
local variable. This is used to not move temporary values since this is 
not required.
2) There are some magic values in flags.

Regards,
Xmilia

diff --git a/lparser.c b/lparser.c
index 3abe3d75..805dfb1e 100644
--- a/lparser.c
+++ b/lparser.c
@@ -1318,8 +1318,67 @@ static void block (LexState *ls) {
 struct LHS_assign {
   struct LHS_assign *prev;
   expdesc v;  /* variable (global, local, upvalue, or indexed) */
+  lu_byte flags;
+  lu_byte tab_replace;
+  lu_byte idx_replace;
 };

+static int search_or_fetch_up (LexState *ls, struct LHS_assign *lh, int 
what) {
+  FuncState *fs = ls->fs;
+  int extra = fs->freereg;
+  for (;lh;lh = lh->prev) {
+    if ((lh->flags & 2) && lh->tab_replace == what) {
+      return lh->v.u.ind.t;
+    }
+  }
+  luaK_codeABC(fs, OP_GETUPVAL, extra, what, 0);
+  luaK_reserveregs(fs, 1);
+  return extra;
+}
+
+static int search_or_fetch_loc (LexState *ls, struct LHS_assign *lh, 
int what) {
+  FuncState *fs = ls->fs;
+  int extra = fs->freereg;
+  for (;lh;lh = lh->prev) {
+    if ((lh->flags & 1) && lh->tab_replace == what) {
+      return lh->v.u.ind.t;
+    }
+    if ((lh->flags & 4) && lh->idx_replace == what) {
+      return lh->v.u.ind.idx;
+    }
+  }
+  luaK_codeABC(fs, OP_MOVE, extra, what, 0);
+  luaK_reserveregs(fs, 1);
+  return extra;
+}
+
+static void prefetch (LexState *ls, struct LHS_assign *lh) {
+  FuncState *fs = ls->fs;
+  int reg;
+
+  lua_assert(vkisindexed(lh->v.k));
+  if (lh->v.k == VINDEXUP) {  /* is table an upvalue? */
+    reg = search_or_fetch_up(ls, lh->prev, lh->v.u.ind.t);
+    lh->flags = 2;
+    lh->tab_replace = lh->v.u.ind.t;
+    lh->v.k = VINDEXSTR;
+    lh->v.u.ind.t = reg;  /* assignment will use safe copy */
+  }
+  else {  /* table is a register */
+    if (lh->v.u.ind.t < fs->nactvar) {
+      reg = search_or_fetch_loc(ls, lh->prev, lh->v.u.ind.t);
+      lh->flags = 1;
+      lh->tab_replace = lh->v.u.ind.t;
+      lh->v.u.ind.t = reg;  /* assignment will use safe copy */
+    }
+    if (lh->v.k == VINDEXED && lh->v.u.ind.idx < fs->nactvar) {
+      reg = search_or_fetch_loc(ls, lh, lh->v.u.ind.idx);
+      lh->flags |= 4;
+      lh->idx_replace = lh->v.u.ind.idx;
+      lh->v.u.ind.idx = reg;
+    }
+  }
+}

 /*
 ** check whether, in an assignment to an upvalue/local variable, the
@@ -1328,39 +1387,22 @@ struct LHS_assign {
 ** use this safe copy in the previous assignment.
 */
 static void check_conflict (LexState *ls, struct LHS_assign *lh, 
expdesc *v) {
-  FuncState *fs = ls->fs;
-  int extra = fs->freereg;  /* eventual position to save local variable */
   int conflict = 0;
-  for (; lh; lh = lh->prev) {  /* check all previous assignments */
-    if (vkisindexed(lh->v.k)) {  /* assignment to table field? */
-      if (lh->v.k == VINDEXUP) {  /* is table an upvalue? */
-        if (v->k == VUPVAL && lh->v.u.ind.t == v->u.info) {
-          conflict = 1;  /* table is the upvalue being assigned now */
-          lh->v.k = VINDEXSTR;
-          lh->v.u.ind.t = extra;  /* assignment will use safe copy */
-        }
-      }
-      else {  /* table is a register */
-        if (v->k == VLOCAL && lh->v.u.ind.t == v->u.var.ridx) {
-          conflict = 1;  /* table is the local being assigned now */
-          lh->v.u.ind.t = extra;  /* assignment will use safe copy */
-        }
-        /* is index the local being assigned? */
-        if (lh->v.k == VINDEXED && v->k == VLOCAL &&
-            lh->v.u.ind.idx == v->u.var.ridx) {
-          conflict = 1;
-          lh->v.u.ind.idx = extra;  /* previous assignment will use 
safe copy */
-        }
-      }
+  lua_assert(vkisindexed(lh->v.k));
+  if (lh->v.k == VINDEXUP) {  /* is table an upvalue? */
+    conflict = v->k == VUPVAL && lh->v.u.ind.t == v->u.info;
+  }
+  else {  /* table is a register */
+    conflict = v->k == VLOCAL && lh->v.u.ind.t == v->u.var.ridx;
+    /* is index the local being assigned? */
+    if (lh->v.k == VINDEXED && v->k == VLOCAL &&
+        lh->v.u.ind.idx == v->u.var.ridx) {
+      conflict = 1;
     }
   }
   if (conflict) {
     /* copy upvalue/local value to a temporary (in position 'extra') */
-    if (v->k == VLOCAL)
-      luaK_codeABC(fs, OP_MOVE, extra, v->u.var.ridx, 0);
-    else
-      luaK_codeABC(fs, OP_GETUPVAL, extra, v->u.info, 0);
-    luaK_reserveregs(fs, 1);
+    prefetch(ls, lh);
   }
 }

@@ -1371,18 +1413,24 @@ static void check_conflict (LexState *ls, struct 
LHS_assign *lh, expdesc *v) {
 ** assignment -> suffixedexp restassign
 ** restassign -> ',' suffixedexp restassign | '=' explist
 */
-static void restassign (LexState *ls, struct LHS_assign *lh, int nvars) {
+static void restassign (LexState *ls, struct LHS_assign *lh, int nvars, 
struct LHS_assign *last_indexed) {
   expdesc e;
   check_condition(ls, vkisvar(lh->v.k), "syntax error");
   check_readonly(ls, &lh->v);
   if (testnext(ls, ',')) {  /* restassign -> ',' suffixedexp restassign */
     struct LHS_assign nv;
-    nv.prev = lh;
     suffixedexp(ls, &nv.v);
-    if (!vkisindexed(nv.v.k))
-      check_conflict(ls, lh, &nv.v);
+    if (vkisindexed(nv.v.k)) {
+      if (last_indexed)
+        prefetch(ls, last_indexed);
+      nv.flags = 0;
+      nv.prev = last_indexed;
+      last_indexed = &nv;
+    } else if (last_indexed) {
+      check_conflict(ls, last_indexed, &nv.v);
+    }
     enterlevel(ls);  /* control recursion depth */
-    restassign(ls, &nv, nvars+1);
+    restassign(ls, &nv, nvars+1, last_indexed);
     leavelevel(ls);
   }
   else {  /* restassign -> '=' explist */
@@ -1798,7 +1846,8 @@ static void exprstat (LexState *ls) {
   suffixedexp(ls, &v.v);
   if (ls->t.token == '=' || ls->t.token == ',') { /* stat -> 
assignment ? */
     v.prev = NULL;
-    restassign(ls, &v, 1);
+    v.flags = 0;
+    restassign(ls, &v, 1, vkisindexed(v.v.k) ? &v : NULL);
   }
   else {  /* stat -> func */
     Instruction *inst;