Re: [Tarantool-patches] [PATCH 2.X 4/7] module api: luaL_register_module & luaL_register_type

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 29.09.2020 07:09, Alexander Turenko wrote:
>>> diff --git a/src/lua/utils.h b/src/lua/utils.h
>>> index 7639cd64a..9b1fe7e57 100644
>>> --- a/src/lua/utils.h
>>> +++ b/src/lua/utils.h
>>> @@ -78,6 +78,8 @@ luaL_pushuuid(struct lua_State *L);
>>>  
>>>  /** \cond public */
>>>  
>>> +struct luaL_Reg;
>>
>> 1. It does not seem to be public. How are users supposed to work with that?
>> Igor, could you please take a look at this?
>>
>> If you need it so badly, at least we need a wrapper around luaL_Reg
>> or we need to expose it and document too. Probably Igor has better ideas.
> 
> It is exposed from lauxlib.h, no need to duplicate in the module API.

Thanks, didn't know it is public. Ok, then no question here.

>>> +
>>>  /**
>>>   * @brief Checks whether a value on the Lua stack is a cdata.
>>>   *
>>> @@ -442,6 +444,8 @@ luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
>>>  	luaL_convertfield(L, cfg, idx, field);
>>>  }
>>>  
>>> +/** \cond public */
>>> +
>>>  void
>>>  luaL_register_type(struct lua_State *L, const char *type_name,
>>>  		   const struct luaL_Reg *methods);
>>
>> 2. These newly exported methods don't have a single comment. Why?
> 
> luaL_register_*() are our helpers around luaL_register(), AFAIR. A
> module should use the latter.

In that case I don't see why do we need to export them.

luaL_register_module() is a wrapper around luaL_register() to
support multilevel modules like 'box.space', 'net.box', etc.
With '.' in them, AFAIS. And a few error checks.

It is not bad to have a bit excessive but commonly used or perf
critical things in module.h, but there is a certain border of
what is "commonly used" and "perf critical". Module registration
does not seem to me fitting any of these categories (subjectively).

So if it can be done without any new exports, I would stick to
that.