Re: Possible closure bug in 4.1 snapshot

[Edgar Toernig <froese@...>]

Roberto Ierusalimschy wrote:
> 
> > First idea: create struct UpVal during OP_CLOSURE, these are linked
> > into lua_State, struct UpVal performs an additional indirection.
> 
> Another solution is to "decompose" the upval address into a base and an
> index.

Should work too.  But I think it's more complicated without being faster.

This would be the relevant routines for my proposal (structures from my
previous posting; assuming GC'ed struct UpVal; untested, may not even
compile!):

lvm.c:
      case OP_GETUPVAL: {
        int b = GETARG_B(i);
        setobj(ra, cl->upvals[b]->valp);
        break;
      }

      case OP_SETUPVAL: {
        int b = GETARG_B(i);
        setobj(cl->upvals[b]->valp, ra);
        break;
      }

      case OP_CLOSURE: {
        ...
        for (j=0; j<nup; j++, pc++) {
          if (GET_OPCODE(*pc) == OP_GETUPVAL)
            ncl->l.upvals[j] = cl->upvals[GETARG_B(*pc)];
          else
            ncl->l.upvals[j] = luaF_localUpVal(L, base + GETARG_B(*pc));
        }
        setclvalue(ra, ncl);
        break;
      }

lfunc.c:
void luaF_localUpVal (lua_State *L, StkId loc)
{
    UpVal **pp, *p;

    for (pp = &L->openupval; (p = *pp) != NULL; pp = &p->u.next)
    {
        if (p->valp == loc)
            return p;
        if (p->valp < loc) 
            break;
    }
    p = luaM_new(L, UpVal);
    p->marked = 0;
    p->next = G(L)->rootupval;
    G(L)->rootupval = p;
    p->valp = loc;
    p->u.next = *pp;
    *pp = p;
    return p;
}


void luaF_close (lua_State *L, StkId downto)
{
    UpVal p;

    while ((p = L->openupval) != NULL && p->valp >= downto)
    {
        L->openupval = p->u.next;
        p->u.value = *p->valp;
        p->valp = &p->u.value;
    }
}


See?  Much simpler.  Uses less memory.  And, as said, I don't think
it's slower.

Ciao, ET.


PS: You can save an additional word per upvalue if you make them
reference counted instead of GC'ed.