Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH v2 2/6] Add functions to ease using Lua iterators from C
Date: Wed, 16 Jan 2019 11:18:02 +0300	[thread overview]
Message-ID: <20190116081802.wpnbjaagm7qftlnb@esperanza> (raw)
In-Reply-To: <20190115232623.huxpqj4adoqqi3mo@tkn_work_nb>

On Wed, Jan 16, 2019 at 02:26:23AM +0300, Alexander Turenko wrote:
> > > diff --git a/src/lua/utils.h b/src/lua/utils.h
> > > index bd302d8e9..6ba2e4767 100644
> > > --- a/src/lua/utils.h
> > > +++ b/src/lua/utils.h
> > > @@ -525,6 +525,34 @@ luaL_checkfinite(struct lua_State *L, struct luaL_serializer *cfg,
> > >  		luaL_error(L, "number must not be NaN or Inf");
> > >  }
> > >  
> > > +/* {{{ Helper functions to interact with a Lua iterator from C */
> > > +
> > > +/**
> > > + * Holds iterator state (references to Lua objects).
> > > + */
> > > +struct luaL_iterator;
> > 
> > I'd make luaL_iterator struct transparent so that one could define it
> > on stack.
> > 
> 
> But luaL_iterator_new() do malloc and return a pointer. So we need to
> change the function or add another one to initialize a structure
> allocated outsize of the module.

OK, I see it now. Didn't think about it when first looked at the patch.

> 
> Can you please suggest me how the API should look if we'll make the
> structure transparent?
> 
> I left it unchanged until we'll discuss this aspect.
> 
> > > +
> > > +/**
> > > + * Create a Lua iterator from {gen, param, state}.
> > 
> > May be, we could pass idx == 0 to create an iterator from
> > gen, param, state (without a table)? Would it be worthwhile?
> > 
> 
> I think it is good idea, because cases could be different. My thought
> was that we'll add another function for this case (if we'll need), but
> using idx == 0 is better. I created the similar API before for
> luaT_newtuple().
> 
> And I think the new merger API will require it.

The patch looks good to me now, but I'm still not sure the new merger
implementation will need to deal with Lua iterators in C at all, as one
can easily write a wrapper function in Lua turning an iterator to a
'fetch' closure, which then can be passed to a source constructor.

  reply	other threads:[~2019-01-16  8:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 20:20 [PATCH v2 0/6] Merger Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 1/6] Add luaL_iscallable with support of cdata metatype Alexander Turenko
2019-01-10 12:21   ` Vladimir Davydov
2019-01-09 20:20 ` [PATCH v2 2/6] Add functions to ease using Lua iterators from C Alexander Turenko
2019-01-10 12:29   ` Vladimir Davydov
2019-01-15 23:26     ` Alexander Turenko
2019-01-16  8:18       ` Vladimir Davydov [this message]
2019-01-16 11:40         ` Alexander Turenko
2019-01-16 12:20           ` Vladimir Davydov
2019-01-17  1:20             ` Alexander Turenko
2019-01-28 18:17         ` Alexander Turenko
2019-03-01  4:04           ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 3/6] lua: add luaT_newtuple() Alexander Turenko
2019-01-10 12:44   ` Vladimir Davydov
2019-01-18 21:58     ` Alexander Turenko
2019-01-23 16:12       ` Vladimir Davydov
2019-01-28 16:51         ` Alexander Turenko
2019-03-01  4:08           ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 4/6] lua: add luaT_new_key_def() Alexander Turenko
2019-01-10 13:07   ` Vladimir Davydov
2019-01-29 18:52     ` Alexander Turenko
2019-01-30 10:58       ` Alexander Turenko
2019-03-01  4:10         ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 5/6] net.box: add helpers to decode msgpack headers Alexander Turenko
2019-01-10 17:29   ` Vladimir Davydov
2019-02-01 15:11     ` Alexander Turenko
2019-03-05 12:00       ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 6/6] Add merger for tuple streams Alexander Turenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190116081802.wpnbjaagm7qftlnb@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH v2 2/6] Add functions to ease using Lua iterators from C' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox