From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D710944643B for ; Sat, 3 Oct 2020 00:48:50 +0300 (MSK) References: <51700ea1fd13c5cbea49e3426551d1c16c1bda02.1600955781.git.tsafin@tarantool.org> <2958f26a-ef0a-4e13-70ec-b99b05cdb2b9@tarantool.org> <20200929055301.udvoq7j4n6fupit6@tkn_work_nb> <20201001030004.755sdofapdsuu77u@tkn_work_nb> <0ade01d698d7$2bf55fa0$83e01ee0$@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Fri, 2 Oct 2020 23:48:49 +0200 MIME-Version: 1.0 In-Reply-To: <0ade01d698d7$2bf55fa0$83e01ee0$@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 2.X 6/7] module api: luaL_checkibuf & luaL_checkconstchar List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Timur Safin , 'Alexander Turenko' Cc: tarantool-patches@dev.tarantool.org On 02.10.2020 18:14, Timur Safin wrote: > : From: Alexander Turenko > : Subject: Re: [Tarantool-patches] [PATCH 2.X 6/7] module api: > : luaL_checkibuf & luaL_checkconstchar > : > : On Wed, Sep 30, 2020 at 01:25:15AM +0200, Vladislav Shpilevoy wrote: > : > On 29.09.2020 07:53, Alexander Turenko wrote: > : > >>> +/** > : > >>> + * Check if a value on @a L stack by index @a idx is an ibuf > : > >>> + * object. Both 'struct ibuf' and 'struct ibuf *' are accepted. > : > >>> + * Returns NULL, if can't convert - not an ibuf object. > : > >>> + */ > : > >>> +struct ibuf * > : > >>> +luaL_checkibuf(struct lua_State *L, int idx); > : > >> > : > >> 1. IMO 'check' is almost always worse than 'is'. Because you leave > : > >> a user no choice but to use lua_cpcall if he does not want an > : > >> exception. With 'is' he would be able to decide whether he wants to > : > >> throw. The same for the method below. > : > > > : > > I personally prefer *is* Lua functions. It is good to have *is* > : variants > : > > in the public API aside of (or even instead of) *check* ones. I don't > : > > insist, however. > : > > : > This is what I said. *is* is better than *check*. More freedom to decide > : > what to do, when it returns false. > : > : I just agreed explicitly. > > Ok, I see the common consensus about naming here, but teher are bigger questions about small reuse... It is not about naming. It is about behaviour. 'is' never throws nor fails. 'check' throws/fails/sets diag. > : > : > > : > I am ok with box_ibuf_t, of any kind. Then we could eventually legally > : expose > : > tarantool_lua_ibuf, as at least for single alloc+free pairs it is better > : than > : > region (a tiny tiny better). > : > > : > > Maybe we can alias with if we'll add static > : > > asserts around the structure size and offset of fields? And expose > : > > box_ibuf_*(), of course. > : > : I meant non-opaque structure and it seems I was not right. We cannot > : alias to now and, when we'll change > : , provide another , which will have the same > : layout as the old . The reason is that 'real' > : is already available via Lua ('buffer' module and via net.box). So if > : we'll going this way (expose non-opaque ), we should typedef > : it to another structure right now and return > : cdata from Lua. It seems, it would be good to > : avoid this extra complexity of code and APIs. > : > : So let's consider opaque approach. It'll add indirect calls > : on hot paths (for each tuple reading from a buffer or writing to a > : buffer), but I don't see any good way to overcome this. Ever increasing > : of tuple's reference counter will be indirect call for an external > : module. > : > : So lot calls to do memcpy() and advance a pointer... > : > : > > : > Also looks fine. However returning to the topic, it seems not related to > : the > : > Lua type ids. You don't need ibuf definition to get its type ID in C. > : All > : > is done in luaL_ctypeid function taking a string anyway. > : > > : > luaL_ctypeid(L, "struct ibuf *"); > : > luaL_ctypeid(L, "struct ibuf"); > : > > : > So why does Timur need that method exported? luaL_checkibuf is a trival > : > function, which can be implemented by luaL_iscdata() + getting ibuf type > : > id using luaL_ctypeid() and comparing result with the cdata type id. > : > : Timur does not need this method, he can implement it on the module side. > : > : But since our built-in module 'buffer' provides cdata and > : cdata, it would be good to have Lua/C accessors in the > : module API. It is simple, so there are no much questions for design, and > : it looks worthful to expose it while we're here. > : > : I would say the same for cdata, but since there is > : another similar cdata type offered by the external key_def module, it > : has no practical reason: we should handle both anyway. > : > : Maybe uuid and decimal too, but I'm not sure. It is hard to estimate how > : many times those functions will be used in modules (that are written by > : us or by users). I would think about this like so: since we provide > : ability to write modules using Lua/C and have cdata objects > : in Lua API, it would be good to have luaT_isfoo() and luaT_pushfoo() > : functions in the module API. > > Exactly so! Maybe. I don't have strong opinion here, so whatever.