NervLuna: Adding support for custom class functions

Hello! Today, we continue with the implementation of the NervLuna bindings system: in our last session on this project we were trying to build some valid bindings for the SimCore project. Everything seems to be working correctly so far on the “standard” bindings generation part. But one thing I eventually realized is that we also definitely need support to be able to add extensions to a given C++ class, that could then be used on the lua side as ordinary functions. So this will be the topic we will focus on this time.

To give you a quick reference example, we are currently relying on some custom functions in the SOL3 bindings system, such as this one:

        SOL_CUSTOM_FUNC(collectRawMessages) = [](class_t& sp, bool clear) { 
            StringList list;
            sp.collectRawMessages(list, clear);
            return sol::as_table(list);
        };

⇒ Basically, in lua, when I build a “SerialPort” object I expect to be able to call a function named “collectRawMessages” on that object, and would retrieve a lua table as result.

For the return mechanism, I think there are basically 2 forms of extensions I should be considering here:

1. Standard return level function extensions: those should take some arguments, and return a type that should be dealt with by the binding system just as with all regular functions (so could be returning a pointer, a struct, a string, etc…)
2. Raw return level functions: those would also take some arguments, but then (also receiving the lua state itself), they will push some results on the stack and return the number of results pushed as a standard lua function.

And now, thinking more about it, I realize that for some very complex functions I might even have to push it one step further and also allow to select between bare arguments or standard arguments processing:

1. For standard arguments, I would process the arguments provided on the lua stack before actually calling the function,
2. But for bare arguments mode, all the function would receive is the lua state itself, and it would be responsible for retrieving arguments from there.

I wondering for instance how I could handle a custom function such as the following one defined in SOL3:

    SOL_CUSTOM_FUNC(sendHexCommand) = sol::overload(
        [](class_t& cmd, const std::string& hstr) { 
            // Execute the command without status callback:
            return cmd.sendHexCommand(hstr); 
        },
        [](class_t& cmd, const std::string& hstr, sol::protected_function callback) { 
            // Execute the command with status callback:
            return cmd.sendHexCommand(hstr, [callback](RCSStatus* status) {
                auto res = callback(status);
                if(!res.valid()) {
                    // An error occured in the processing:
     	            sol::error err = res;
		            std::string what = err.what();
		            logERROR("Error in status handler: "<<what);     
                }
            });
        });

In the second overload above, the function will receive a callback function defined in lua, it should somehow keep a reference on it, and eventually call it when required… Hmmm, this is all starting to sound a bit tricky 😆.

⇒ From my current perspective, the “most correct” option to deal with this setup would be to accept the lua_state as argument, and get the index where the function is stored, and keep a reference in the lua registry on that index. And eventually, we would retrive the “function reference” and use it. But not quite sure we really have to keep a reference actually (?)…

As we have to start somewhere, I simply wrote the following simple extension function

inline int __lunarawext__collectRawMessages(SerialPort& self, lua_State *L, bool clear = true)
{
    StringList list;
    self.collectRawMessages(list, clear);

    // build a lua table:
    lua_newtable(L);
    int i = 1;
    for(auto& str : list) {
        lua_pushlstring(L, str.data(), str.size());
        lua_rawseti(L, -2, i++);
    }

    return 1;
}

The idea here is that I will use the prefix __lunarawext__ to declare that this function should be considered an “extension” to the class provided as first argument and that it should use the “raw” return mechanism I described above.

And at the same time, since I have the lua_state* as one of the function arguments, this state will be implicitly passed to the function call, and might be used to collect additional inputs and/or push results on the stack. Let's now have some fun an try to generate the bindings for this 🤣:

nv_seed bind4 | tee bind4.log

Hmm, okay, so this was producing some diagnostic errors during parsing:

[Info] 	   Num diagnostics for file D:\Projects\SimCore\sources\lua_modules\luamx\luna_bindings.h: 4
[Info] 	    - diag 1: D:\Projects\SimCore\sources\lua_modules\luamx\include\shared_context.h:29:5: error: unknown type name 'StringList'
[Info] 	    - diag 2: D:\Projects\SimCore\sources\lua_modules\luamx\include\shared_context.h:30:10: error: no member named 'collectRawMessages' in 'mx::SerialPort'
[Info] 	    - diag 3: D:\Projects\SimCore\sources\lua_modules\luamx\include\shared_context.h:33:5: error: use of undeclared identifier 'lua_newtable'
[Info] 	    - diag 4: D:\Projects\SimCore\sources\lua_modules\luamx\include\shared_context.h:37:9: error: use of undeclared identifier 'lua_rawseti'

And this makes sense since I'm currently only including the bare minimals during binding pass. And for now, I think i should keep it that way, so we will separate the function declaration and implementation here (using compile_context.h and parse_context.h instead of shared_context.h)

Now in parse_context.h we only have the following line:

LUNA_IMPORT int __lunarawext__collectRawMessages(SerialPort& self, lua_State *L, bool clear = true);

And bindings generation is now going fine with no diagnostics.

Let's check the generated function:

// Bind for __lunarawext__collectRawMessages (1) with signature: int (mx::SerialPort &, lua_State *, bool)
static int _bind_mx__lunaraw_collectRawMessages_sig1(lua_State* L) {
	int luatop = lua_gettop(L);
	mx::SerialPort* self = Luna< mx::SerialPort >::get(L,1,false);
	lua_State* L = Luna< lua_State >::get(L,2,true);
	bool clear = luatop>=3? (bool)lua_toboolean(L,3) : (bool)true;

	int res = mx::__lunarawext__collectRawMessages(*self, L, clear);
	lua_pushinteger(L,res);

	return 1;
}

This will not compile obviously since we are redefining the lua_State* L variable: we need to ensure that the system will ignore retrieve that type of variable from the stack. For this, the following code snippet in my FunctionWritter seems to do the trick

  local argnames = {}
  local argIdx=1
  for i=1,num do
    local arg = sig:getArgument(i)
    local conv = arg:getLuaConverter()
    
    -- If that argument is a lua_State then we should not try to retrieve it from the stack at all:
    if arg:getName()=="L" and arg:getType():getName()=="lua_State*" then
      logDEBUG("Detected lua state argument in signature: ", sig:getName())
      -- This is the lua_state so we simply push the name "L" as function call argument:
      table.insert(argnames, "L")
    else
      -- We get the argument from lua entry "i"
      -- and we store the argument name at the same time:
      table.insert(argnames, conv:writeGetter(self, argIdx+argOffset, arg))
      argIdx = argIdx + 1
    end
  end</sxhjs>

But then the function parameters checker also needs fixing, let's see... Again the changes are somewhat minimal: <sxhjs lua>
  -- @luna4: if one of the argument is lua_State* L then we should remove 1 to the argOffset:
  if sig:hasLuaStateArgument() then
    argOffset = argOffset - 1
  end


  -- We check the args with condition after the first min args:
  local argIdx = 1
  for i=1,maxargs do
    local arg = sig:getArgument(i)
    
    if not arg:isCanonicalLuaState() then
      -- retrieve the lua check type corresponding to that argument:
      local conv = arg:getLuaConverter()

      conv:writeArgCheck(self, argIdx+argOffset, arg)
      argIdx = argIdx + 1
    end
  end
</sxhjs>

=> Okay, cool! With those changes, the compilation seems to be fine! 😊 Yet, that's only the first step required here.


===== Integrating extension into the parent class =====

As I said above, compilation is now OK, but the actual behavior is not what I want: <sxh cpp; highlight: []>// Bind for __lunarawext__collectRawMessages (1) with signature: int (mx::SerialPort &, lua_State *, bool)
static int _bind_mx__lunaraw_collectRawMessages_sig1(lua_State* L) {
	int luatop = lua_gettop(L);
	mx::SerialPort* self = Luna< mx::SerialPort >::get(L,1,false);
	bool clear = luatop>=2? (bool)lua_toboolean(L,2) : (bool)true;

	int res = mx::__lunarawext__collectRawMessages(*self, L, clear);
	lua_pushinteger(L,res);

	return 1;
}

In the code above, I do not want to push an integer on the stack: instead the integer value I'm receiving from the function call is the number of results already pushed on the stack, so this is what I should be returning directly when dealing with a lunarawext function.

Yet, before I could handle this part correctly I first had to ensure the function was correctly relocated into its corresponding parent class. So here is the post processing I added at the end of my “parseFunction” function:

  -- Once we are done parsing this function we can figure out if this is a __lunarawext__ extension
  local fname = func:getName()
  if fname:startsWith("__lunarawext__") then
     fname = fname:sub(15)

    -- retrieve the class corresponding to the first argument of the function:
    local arg = sig:getArgument(1)
    local atype = arg:getType()
    -- Should be a reference:
    CHECK(atype:isReference() , "Invalid base type for luna extension:")
    -- Get the reference class:
    local btype = atype:getBaseType()
    -- local cl = luna:getClass(btype)
    local cl = btype:getTarget()
    CHECK(cl~=nil, "Cannot retrieve base class for extension sig:", sig:getName())
    
    -- Also, the return type of the sig should be "int":
    CHECK(sig:getReturnType():isInt32(), "Invalid return type for raw extension function ", fname)

    logDEBUG("Detected luna raw extension: ",fname, " for class ", cl:getFullName())
    -- So now we should get or create this function in the class itself:
    local extFn = cl:getOrCreateFunction(fname)
    
    -- Should not already have that extension signature:
    CHECK(not extFn:hasSignature(signame), "raw extension ", signame," was already registered.")
    sig:reparentTo(extFn)

    -- We should discard the first argument from the signature now:
    sig:removeArgument(1)
  end</sxhjs>

With that code, when I'm done parsing a free function with a name starting with ''__lunarawext__'' I can move the newly generated function signature into the correct class. And this will give be the following kind of result: <sxh cpp; highlight: []>// Bind for collectRawMessages (1) with signature: int (mx::SerialPort &, lua_State *, bool)
static int _bind_collectRawMessages_sig1(lua_State* L) {
	int luatop = lua_gettop(L);
	mx::SerialPort* self = Luna< mx::SerialPort >::get(L,1);
	LUNA_ASSERT(self!=nullptr);

	bool clear = luatop>=2? (bool)lua_toboolean(L,2) : (bool)true;

	int res = self->collectRawMessages(L, clear);
	lua_pushinteger(L,res);

	return 1;
}

// Overall bind for collectRawMessages
static int _bind_collectRawMessages(lua_State* L) {
	if(_check_collectRawMessages_sig1(L)) return _bind_collectRawMessages_sig1(L);

	LUNA_ERROR_MSG("Current lua stack: "<<luna_dumpStack(L));
	luaL_error(L, "Binding error for function collectRawMessages, cannot match any of the 1 signature(s):\n  sig1: int (mx::SerialPort &, lua_State *, bool)");
	return 0;
}

Not too bad, except that now I should be calling the free function itself, instead of trying to call the non-existing member function collectRawMessages. Let's deal with that.

⇒ So we handle that in the FunctionWriter class with the code snippet:

  -- if we have a method then we should rely on the "self" object instead:
  if isMethod and not isStatic then
    if sig:isExtension() then
      funcName = sig:getExtFullName()
      table.insert(argnames, 1, "*self")
      callstr = table.concat{"(", table.concat(argnames, ", "), ")"}
    else
      funcName = "self->" .. func:getName()
    end
  end</sxhjs>

And this will now produce this kind of code: <sxh cpp; highlight: []>// Bind for collectRawMessages (1) with signature: int (mx::SerialPort &, lua_State *, bool)
static int _bind_collectRawMessages_sig1(lua_State* L) {
	int luatop = lua_gettop(L);
	mx::SerialPort* self = Luna< mx::SerialPort >::get(L,1);
	LUNA_ASSERT(self!=nullptr);

	bool clear = luatop>=2? (bool)lua_toboolean(L,2) : (bool)true;

	int res = mx::__lunarawext__collectRawMessages(*self, L, clear);
	lua_pushinteger(L,res);

	return 1;
}

And this code will compile 😊! But at this point I still need to handle the raw return mechanism.

And here too just adding a simple flag on the signature when we read the lunarawext prefix seems to be enough to generate the correct code afterwards:

    -- Otherwise we need to get a return:
    self:writeLine("%s res = %s%s;", rtype:getName(), funcName, callstr)
    
    -- @luna4: check here if we should return raw results or not:
    if sig:getRawReturns() then
      self:newLine()
      self:writeLine("return res;") 
    else
      local conv = rtype:getLuaConverter()
      CHECK(conv, "Invalid converter for type: ", rtype)
      conv:writeSetter(self, "res", rtype, "");
      -- This time we have one result to return:
      self:newLine()
      self:writeLine("return 1;") 
    end</sxhjs>

And the generated code is now as follow for our custom function: <sxh cpp; highlight: []>// Bind for collectRawMessages (1) with signature: int (mx::SerialPort &, lua_State *, bool)
static int _bind_collectRawMessages_sig1(lua_State* L) {
	int luatop = lua_gettop(L);
	mx::SerialPort* self = Luna< mx::SerialPort >::get(L,1);
	LUNA_ASSERT(self!=nullptr);

	bool clear = luatop>=2? (bool)lua_toboolean(L,2) : (bool)true;

	int res = mx::__lunarawext__collectRawMessages(*self, L, clear);

	return res;
}

… and this is compiling! All right! ✌

We then continue our journey trying to add support for lua callbacks directly inside our custome functions:

inline int __lunarawext__sendCommand(Commander& cmd, lua_State *L, unsigned int cmdId)
{
    using namespace luna;

    int luatop = lua_gettop(L);

    // Retrieve the function from the top of the stack:
    LUNA_ASSERT(luatop==2);
    LUNA_ASSERT(lua_isfunction(L,2));

    bool res = cmd.sendCommand(cmdId, [L](RCSStatus* status) {

        // push the function on the stack:
        lua_pushvalue(L, 2);
        Luna< RCSStatus >::push(L, status, false);

        if (lua_pcall(L, 1, 0, 0) != 0) {
            // Error occurred:
            logERROR("Error in RCSStatus handler: "<<lua_tostring(L, -1));
            lua_pop(L,1);
        }
    });

    // Push the bool result:
    lua_pushboolean(L, res);
    return 1;
}

The code generated for that function seems legit (but not tested yet): but at the same time I'm thinking I could try to make it a bit smaller/easier to write by adding a class encapsulation for the “lua function” objects…

So I introduced the following lua function helper class:

    template<typename T>
    void pushValue(lua_State* L, T val) {
        LUNA_THROW_MSG("Calling default implementation for pushValue.")
    }

    template <typename T>
    void pushValue(lua_State* L, const T* arg) {
        Luna< T >::push(L, arg, false);
    };

    inline void pushValue(lua_State* L, bool arg) {
        lua_pushboolean(L,arg?1:0);
    }

    inline void pushValue(lua_State* L, lua_Integer arg) {
        lua_pushinteger(L,arg);
    }

    inline void pushValue(lua_State* L, double arg) {
        lua_pushnumber(L,arg);
    }

    struct LuaFunction
    {
        lua_State* state{nullptr};
        int index{0};
        int nargs{0};
        int nresults{0};

        LuaFunction() {};
        LuaFunction(lua_State* L, int idx) : state(L), index(idx) {
            LUNA_ASSERT(lua_isfunction(state,index));
        };

        LuaFunction& numResults(int num) { nresults = num; return *this; }

        void reset() { nargs = 0; }

        bool valid() const {
            return state!=nullptr && index!=0;
        }

        LuaFunction& pushSelf() {
            // Check validity:
            LUNA_ASSERT(valid());

            // Clear state:
            reset();

            // Push the function itself:
            lua_pushvalue(state, index);

            return *this;
        };

        template <typename T>
        LuaFunction& pushArg(T arg) {
            ++nargs;
            pushValue(state, arg);
            return *this;
        };
        
        bool execute() const {
            if (lua_pcall(state, nargs, nresults, 0) != 0) {
                // Error occurred:
                LUNA_ERROR_MSG("Exception in LuaFunction execution: "<<lua_tostring(state, -1));
                lua_pop(state,1);
                return false;
            }

            return true;
        }

        bool operator()() {
            pushSelf();
            return execute();
        }

        template<typename U1>
        bool operator()(U1 arg1) {
            pushSelf();
            pushArg(arg1);
            return execute();
        }
        template<typename U1, typename U2>
        bool operator()(U1 arg1, U2 arg2) {
            pushSelf();
            pushArg(arg1);
            pushArg(arg2);
            return execute();
        }
        template<typename U1, typename U2, typename U3>
        bool operator()(U1 arg1, U2 arg2, U3 arg3) {
            pushSelf();
            pushArg(arg1);
            pushArg(arg2);
            pushArg(arg3);
            return execute();
        }
        template<typename U1, typename U2, typename U3, typename U4>
        bool operator()(U1 arg1, U2 arg2, U3 arg3, U4 arg4) {
            pushSelf();
            pushArg(arg1);
            pushArg(arg2);
            pushArg(arg3);
            pushArg(arg4);
            return execute();
        }
    };

With this, I could then add the corresponding “LuaFunctionConverter” and eventually upgrade my custom function to directly expect a LuaFunction object as parameter:

inline int __lunarawext__sendCommand(Commander& cmd, lua_State* L, unsigned int cmdId, LuaFunction* fn = nullptr, bool forceAck = false)
{
    bool res = cmd.sendCommand(cmdId, [fn](RCSStatus* status) {
        (*fn)(status);
    });

    // Push the bool result:
    pushValue(L, res);
    return 1;
}

And again, code generation and compilation both worked pretty much without any serious trouble… [strangely… ? 🤔]

And now given the lastest version of my custom function definition above, the next logical step I see is actually to get rid of the “raw return” mechanism in this specific case: indeed here, I only need access to the lua state to push a single bool value on it. So I might as well just return that bool and let the regular binding layer handling pushing the value on stack as usual.

⇒ Simply updating the handling of the extension function creation with the following to support both types of extensions:

  local fname = func:getName()

  local isExtension = false
  local isRawExt = false

  if fname:startsWith("__lunarawext__") then
     fname = fname:sub(15)
     isExtension = true
     isRawExt = true
  end

  if fname:startsWith("__lunaext__") then
     fname = fname:sub(12)
     isExtension = true
  end

  if isExtension then
    -- retrieve the class corresponding to the first argument of the function:
    local arg = sig:getArgument(1)
    local atype = arg:getType()
    -- Should be a reference:
    CHECK(atype:isReference() , "Invalid base type for luna extension:")
    -- Get the reference class:
    local btype = atype:getBaseType()
    -- local cl = luna:getClass(btype)
    local cl = btype:getTarget()
    CHECK(cl~=nil, "Cannot retrieve base class for extension sig:", sig:getName())
    
    logDEBUG("Detected luna extension: ",fname, " for class ", cl:getFullName())
    -- So now we should get or create this function in the class itself:
    local extFn = cl:getOrCreateFunction(fname)
    
    -- Should not already have that extension signature:
    CHECK(not extFn:hasSignature(signame), " extension ", signame," was already registered.")
    sig:reparentTo(extFn)

    -- We should discard the first argument from the signature now:
    sig:removeArgument(1)

    sig:setExtFullName(func:getFullName())
    
    if isRawExt then
      -- Also, the return type of the sig should be "int":
      CHECK(sig:getReturnType():isInt32(), "Invalid return type for raw extension function ", fname)
      sig:setRawReturns(true)
    end
  end</sxhjs>

And now I can in finally remove the lua_State parameter completely and thus simplify the custom function a little bit further: <sxh cpp; highlight: []>inline bool __lunaext__sendCommand(Commander& cmd, unsigned int cmdId, luna::LuaFunction* fn = nullptr, bool forceAck = false)
{
    auto cb = fn ? [fn](RCSStatus* status) { (*fn)(status); } : nullptr;
    return cmd.sendCommand(cmdId, cb, forceAck);
}

Pretty minimal now if you ask me 😁!

Okay, so with those new bindings, I was finally ready to start testing the execution of some lua code using them! And there of course, I got some problems 😁.

First issue was apparently on how I create my callback in the sendCommand custom function, so I had to use something like this instead of the 2 lines shown above:

inline bool __lunaext__sendCommand(Commander& cmd, unsigned int cmdId, luna::LuaFunction* fn = nullptr, bool forceAck = false)
{
    Commander::StatusFunc cb = nullptr;
    if(fn) {
        cb = [fn](RCSStatus* status) { (*fn)(status); };
    }
    return cmd.sendCommand(cmdId, cb, forceAck);
}

Second issue was when actually trying to push an RCSStatus pointer on the stack before calling the lua function itself. I got an exception like that:

d:\agent\_work\2\s\src\vctools\crt\vcruntime\src\eh\throw.cpp (133): _CxxThrowException
d:\projects\simcore\deps\msvc64\nvluna-0.1\include\lua\luna.h (713): luna::pushValue<mx::RCSStatus *>
d:\projects\simcore\deps\msvc64\nvluna-0.1\include\lua\luna.h (782): luna::LuaFunction::pushArg<mx::RCSStatus *>
d:\projects\simcore\deps\msvc64\nvluna-0.1\include\lua\luna.h (805): luna::LuaFunction::operator()<mx::RCSStatus *>
d:\projects\simcore\sources\lua_modules\luamx\include\compile_context.h (88): <lambda_c2fe06b66791108363208aa2f91c7485>::operator()

⇒ That one was because I was trying to call the default implementation of pushValue, and this will throw an exception by design:

template<typename T>
    void pushValue(lua_State* L, T val) {
        LUNA_THROW_MSG("Calling default implementation for pushValue.")
    }

In fact, I should simply just remove that default implementation and only provide an implementation to deal with non const pointers instead: so I would get failures at compile time instead (if no matching version is found by the compiler):

    template<typename T>
    void pushValue(lua_State* L, T* arg) {
        Luna< T >::push(L, arg, false);
    }

And with that final change: bingo! My simple lua test script works perfectly fine! Yeepee! I was definitely not asking for more that this, so I think this is now a good time to stop this little emprovement session and enjoy a well deserved break😝!

See you another time everyone for even more funny “binding adventures”