NervLuna: Handling C++ operators

In this new development session on NervLuna we will focusing on providing the correct support for the operator bindings and also fix some regression issues due to our latest updates on the parsing system. Let's get started!

Most significant issue I noticed when trying to re-generate the binding code for our minimal test module with the lastest version of the generator, is the handling of the “std::string”: initially I was specifying a “manually provided” lua converter for a type called “std::string” that I was also manually adding before parsing anything. This was working fine, but then I had to remove that part to let libclang really find by itself what “std::string” is and why it should be considered the same thing as some other classes (ie. remember: “std::basic_string<char, std::char_traits<char>, std::allocator<char>>” from my previous post on this topic)

So previous code such as:

// Bind for setString (1) with signature: void (const std::string &)
static int _bind_setString_sig1(lua_State* L) {
	// When reaching this call, we assume that the type checking is already done.
	nvt::MyClass* self = Luna< nvt::MyClass >::get(L,1);
	ASSERT(self!=nullptr);

	size_t str_len = 0;
	const char* str_cstr = lua_tolstring(L,2,&str_len);
	std::string str(str_cstr, str_len);

	self->setString(str);

	return 0;
}

Is currently replaced with simply:

static int _bind_setString_sig1(lua_State* L) {
	// When reaching this call, we assume that the type checking is already done.
	nvt::MyClass* self = Luna< nvt::MyClass >::get(L,1);
	ASSERT(self!=nullptr);

	std::string* str = Luna< std::string >::get(L,2,false);

	self->setString(*str);

	return 0;
}

⇒ Not quite what we need/want for a string class . So let's find how to retore the previous lua converter somehow.

OK: this was easy, I just had to update my TypeManager:getLuaConverter() function as follow:

function Class:getLuaConverter(tgt)
  if type(tgt)=="string" then
    return self._converters[tgt]
  end

  -- otherwise the the target is a "class"
  -- so we should be able to retrieve its full name:
  local fname = tgt:getFullName()
  local conv = self._converters[fname]
  if conv then
    -- logDEBUG("Using custom converter for class: ", fname)
    return conv
  end

  -- No converter found
  return nil
end</sxhjs>

And now I'm back to the correct handling of the std::string parameters.

<note>The generator will still generate/register a class binding file for std::string. Technically I should not need that, since I'm not pushing/retrieving any instance of that class from the lua stack. But for now, it doesn't hurt to have it here anyway, so i'm leaving this as is.</note>

===== Handling of operator functions =====

And now, just trying to rebuild my test module, we finally reach the main issue to be discussed here: the handling of operators. At some point I added the following function to be binded in the test module header file:

<sxh cpp>
namespace std {
inline std::ostream& operator<< (std::ostream& os, const nvt::Point& pt)
{
  os << "nvt::Point(" << pt.x << ", " << pt.y << ")";
  return os;
}
}

So from that, I currently get the following bindings generated:

// Typecheck for operator<< (1) with signature: std::ostream &(std::ostream &, const nvt::Point &)
static bool _check_operator<<_sig1(lua_State* L) {
	int luatop = lua_gettop(L);
	if( luatop!=2 ) return false;

	if( !luna_isInstanceOf(L,1,SID("std::ostream"),false) ) return false;
	if( !luna_isInstanceOf(L,2,SID("nvt::Point"),false) ) return false;
	return true;
}

// Bind for operator<< (1) with signature: std::ostream &(std::ostream &, const nvt::Point &)
static int _bind_operator<<_sig1(lua_State* L) {
	// When reaching this call, we assume that the type checking is already done.
	std::ostream* os = Luna< std::ostream >::get(L,1,false);
	nvt::Point* pt = Luna< nvt::Point >::get(L,2,false);

	std::ostream & res = std::operator<<(*os, *pt);
	Luna< std::ostream >::push(L, &res, false);

	return 1;
}

// Overall bind for operator<<
static int _bind_operator<<(lua_State* L) {
	if(_check_operator<<_sig1(L)) return _bind_operator<<_sig1(L);

	luaL_error(L, "Binding error for function operator<<, cannot match any of the 1 signature(s):\n  sig1: std::ostream &(std::ostream &, const nvt::Point &)");
	return 0;
}

void register_functions(lua_State* L) {
  // More stuff here.

	nv::luna_pushNamespace(L,{"std"});

	lua_pushcfunction(L, _bind_operator<<); lua_setfield(L,-2,"operator<<");

	nv::luna_popNamespace(L);
}

Obviously, all the highlighted lines in the code snippet above will refuse to compile So let's investigate how to deal with this…

To start with, there is one nasty issue I didn't expect in my binding generation system: renaming a function from “C++ name” to “lua name” would not be that hard. But to be able to deal with overloading correctly I'm currently “grouping” all the signatures found for a given C++ function name inside the same function object.

Yet, here for instance, if we take the “operator-” function name: in C++ we could have an overload taking 1 parameter, and another taking 0 parameter (for the “unary_minus”)… and in lua, we should use the metamethod name “__add” in the first case, and “__unm” in the other case :-S.

⇒ So I need to figure out how I could cut my function into “2 separated copies” in this specific case…

But, first, let's handle the generic case, where we simply need to rename the C++ operator. For this I simply added a new function “getLuaName()” to the class used to represent a function as follow:

-- cf. https://stackoverflow.com/questions/8679089/c-official-operator-names-keywords
-- cf. http://lua-users.org/wiki/MetatableEvents


local foreignOperatorMap = {
  ["operator++"] = "op_post_inc",
  ["operator--"] = "op_post_dec",
  ["operator++"] = "op_pre_inc",
  ["operator--"] = "op_pre_dec",
  ["operator!"] = "op_neg",
  ["operator~"] = "op_comp",
  ["operator&"] = "op_address_of",
  ["operator!="] = "op_not_equal",
  ["operator>"] = "op_gt",
  ["operator>="] = "op_ge",
  ["operator&&"] = "op_and",
  ["operator||"] = "op_or",
  ["operator&"] = "op_bitand",
  ["operator|"] = "op_bitor",
  ["operator<<"] = "op_lshift",
  ["operator>>"] = "op_rshift",
  ["operator+="] = "op_add_assign",
  ["operator-="] = "op_sub_assign",
  ["operator*="] = "op_mul_assign",
  ["operator/="] = "op_div_assign",
  ["operator%="] = "op_mod_assign",
  ["operator>>="] = "op_rshift_assign",
  ["operator<<="] = "op_lshift_assign",
  ["operator&="] = "op_bitand_assign",
  ["operator^="] = "op_bitxor_assign",
  ["operator|="] = "op_bitor_assign",
  ["operator->*"] = "op_arrow_ind",
  ["operator,"] = "op_comma",
  ["operator="] = "op_assign",
  ["operator[]"] = "op_get",
  ["operator->"] = "op_arrow",
  ["operator."] = "op_dot",
  ["operator.*"] = "op_dot_ind",
}

function Class:getLuaName()
  -- #112: special handling if the function is actually an operator:
  -- If this is simply a "foreign operator" then we simply change its name here:
  local fname = self:getName()
  local cppname = foreignOperatorMap[fname]
  if cppname then
    return cppname
  end

  -- Otherwise we just return the actual function name:
  return fname
end
</sxhjs>

=> Then using this new function works just fine to retrieve an appropriate name for the function in lua. And thus with a few additional changes, I could get my test module to compile again: yeeeppeee LOL


===== Handling actual metamethods =====

Now it's time to move to the (slightly?) more complex case: when should I consider that a "C++ operator" is a valid [[http://lua-users.org/wiki/MetatableEvents|lua metamethod]] ?

The point is, the C++ operators might sometimes have different overloads with different number of signatures, and yet, only one of those signatures would actually be valid to be used as a metamethod (cf. the discussion on the "unary_minus" operator above. But this is also applicable to other operators such as "+" or "*").

=> So I think I need to make the function "lua name" dependent on the signature we are currently considering (?). Let's see if this could work... 

For testing, I'm adding the following operators in one of my test class:

<sxh cpp>
struct Point
{
    float x{0.0f};
    float y{0.0f};

    Point() {};
    Point(float xx, float yy) : x(xx), y(yy) {};

    Point operator+(const Point& rhs) {
        return {x+rhs.x, y+rhs.y};
    }

    Point operator-(const Point& rhs) {
        return {x-rhs.x, y-rhs.y};
    }

    Point operator-() {
        return {-x, -y};
    }

    Point operator*(float m) {
        return {x*m, y*m};
    }

    float operator*() {
        return x;
    }
};

Point operator+(const Point& a, const Point& b);

And now I'm extending the getLuaName() function as follow:

local compatOperatorMap = {
  ["operator+"] =  { [1] = "__add", [0] = "op_plus", def = "op_add" }, 
  ["operator-"] =  { [1] = "__sub", [0] = "__unm", def = "op_sub" },
  ["operator*"] =  { [1] = "__mul", [0] = "op_ind", def = "op_mul" },
  ["operator/"] =  { [1] = "__div", def = "op_div" },
  ["operator=="] = { [1] = "__eq", def = "op_eq" },
  ["operator%"] =  { [1] = "__mod", def = "op_mod" }, 
  ["operator^"] =  { [1] = "__pow", def = "op_pow" }, 
  ["operator<"] =  { [1] = "__lt", def = "op_lt" },
  ["operator<="] = { [1] = "__le", def = "op_le" },
  ["operator()"] = { def = "__call" },
}

function Class:getLuaName(sig)
  -- #112: special handling if the function is actually an operator:
  -- If this is simply a "foreign operator" then we simply change its name here:
  local fname = self:getName()
  local cppname = foreignOperatorMap[fname]
  if cppname then
    return cppname
  end

  -- Now, this might be one of the possible metamethod signature so we should check that:
  local comp = compatOperatorMap[fname]
  if comp then
    -- We retrieve the name that will correspond to the number of arguments for the given signature:
    -- or the default name:
    return comp[sig:getNumArguments()] or comp.def
  end

  -- Otherwise we just return the actual function name:
  return fname
end
</sxhjs>

In the binding generation itself, I'm also making sure that for each function I process I point each given signature to the appropriate "lua name", and finally, I register all the lua names that would correspond to a given function, with this kind of code:

<sxhjs lua>
  for _,f in ipairs(pubFuncs) do
    if not f:isConstructor() and not f:isDestructor() then
      -- We should collect all the possible luaNames for that function:
      local lnames = f:getLuaNames()
      for _,lname in ipairs(lnames) do
        self:writeSubLine("{\"${1}\", &_bind_${1}},", lname)
      end
    end
  end
</sxhjs>

And **OH MY...**! the results look great:

<sxh cpp>
// Bind for operator- (1) with signature: nvt::Point (const nvt::Point &)
static int _bind___sub_sig1(lua_State* L) {
	// When reaching this call, we assume that the type checking is already done.
	nvt::Point* self = Luna< nvt::Point >::get(L,1);
	ASSERT(self!=nullptr);

	nvt::Point* rhs = Luna< nvt::Point >::get(L,2,false);

	nvt::Point res = self->operator-(*rhs);
	nvt::Point* res_ptr = new nvt::Point(res);
	Luna< nvt::Point >::push(L, res_ptr, true);

	return 1;
}

// Bind for operator- (2) with signature: nvt::Point ()
static int _bind___unm_sig2(lua_State* L) {
	// When reaching this call, we assume that the type checking is already done.
	nvt::Point* self = Luna< nvt::Point >::get(L,1);
	ASSERT(self!=nullptr);


	nvt::Point res = self->operator-();
	nvt::Point* res_ptr = new nvt::Point(res);
	Luna< nvt::Point >::push(L, res_ptr, true);

	return 1;
}

Compilation is also going just fine! So, now time for a little unit test on this, starting with additions and subtractions:

describe("Usage of simple operators", function()

  it("Should support using the operator+ metamethod", function()

      local p1 = nvt.Point()
      p1.x = 1
      p1.y = 2
      local p2 = nvt.Point()
      p2.x = 3
      p2.y = 4

      assert_equal(p1.x, 1)
      assert_equal(p1.y, 2)
      assert_equal(p2.x, 3)
      assert_equal(p2.y, 4)

      local p3 = p1 + p2

      assert_equal(p3.x,4)
      assert_equal(p3.y,6)

      -- Call the global add operator:
      local p4 = nvt.op_add(p3,p1)
      assert_equal(p4.x,5)
      assert_equal(p4.y,8)
  end)

  it("Should support using the operator- metamethod", function()

      local p1 = nvt.Point(1,2)
      local p2 = nvt.Point(3,4)
      assert_equal(p1.x, 1)
      assert_equal(p1.y, 2)
      assert_equal(p2.x, 3)
      assert_equal(p2.y, 4)

      local p3 = p1 - p2

      assert_equal(p3.x,-2)
      assert_equal(p3.y,-2)

      local p4 = -p1
      assert_equal(p4.x,-1)
      assert_equal(p4.y,-2)

  end)

end)
</sxhjs>

=> In the test above everything went fine **except** for the unary_minus operator test: this one doesn't work because I'm receiving one additional element on the stack that I'm not expecting (ie. the input object itself):

<code>
[Error]         Current stack: stack trace: top 2
  1: [object] ptr=00000160FD338C40, ID=1222735628657379899 (=nvt.Point), ids=[], gc=true
  2: [object] ptr=00000160FD338C40, ID=1222735628657379899 (=nvt.Point), ids=[], gc=true

[Error]         FAILURE in test 'Should support using the operator- metamethod' in context 'Usage of simple operators'
</code>

But still, the definition of the lua %%__unm%% function (from http://lua-users.org/wiki/MetatableEvents) says:

> %%__unm - Unary minus. When writing "-myTable", if the metatable has a __unm key pointing to a function, that function is invoked (passing the table), and the return value used as the value of "-myTable".%%

So I was really expecting to see only 1 argument on the stack at this point... Yet, after some additional investigation I found this [[http://lua-users.org/lists/lua-l/2010-10/msg00799.html|discussion thread]] which acknowledge that we are indeed receive twice the same argument:

> You are not missing anything. This is only a hack. By adding this extra argument we can use for these metamethods the same machinery already in place for all other binary metamethods :) [Roberto Ierusalimschy]

Simply using the folling additional code when generating the function type checks is enough to fix this problem:

<sxhjs lua> --%%
  -- #112: special handling of the __unm function: lua will automatically push the "table" (ie userdata for us) *twice*
  -- on the stack before calling this function, so we must take that into account here:
  if lname == "__unm" then
    argOffset = argOffset + 1
  end
--%%
</sxhjs>

===== Multiplications =====

Next I continue with the test for operator*, and here also we have an interesting little surprise: in the code below, the test with the "p3" object will go just fine, but for p4 we get an error, because the first argument on the stack in that case is a number, and not a "Point" object as I would expect:

<sxhjs lua>
  it("Should support using the operator* metamethod", function()
      local p1 = nvt.Point(1,2)
      local p3 = p1 * 3

      assert_equal(p3.x,3)
      assert_equal(p3.y,6)

      local p4 = 2 * p1
      assert_equal(p4.x,2)
      assert_equal(p4.y,4)
  end)
</sxhjs>

As a result of this, for the lua metamethods %%__add%% and %%__mul%% I'm now adding support to automatically switch the order of the arguments if the first argument on the stack doesn't have the correct type, but the second does, thus producing this kind of binding code:

<sxh cpp; highlight: [8-11]>
// %%
// Typecheck for operator* (1) with signature: nvt::Point (float)
static bool _check___mul_sig1(lua_State* L) {
	int luatop = lua_gettop(L);
	if( luatop!=2 ) return false;

	// Support for auto swapping arguments for __mul:
	if(!luna_isInstanceOf(L,1,SID("nvt::Point")) && luna_isInstanceOf(L,2,SID("nvt::Point"))) {
		lua_pushvalue(L,1);
		lua_remove(L,1);
	}

	if( lua_isnumber(L,2)!=1 ) return false;
	return true;
}
// %%

⇒ With that kind of code the unit test above is now passing.

And I think this will be it for today I was able to rebuild successfully my test module and the Vulkan module with the latest updates above. Concerning the nvCore module, it's still not building unfortunately But the number of errors observed there is reducing quickly, so I still have hope! Right now, it seems the C++ operators handling mechanism is doing its job, and the other errors I'm left with don't seem to be directly related to it.

Actually, the next thing I can notice going wrong is due to this binding function:

// Bind for LogSink constructor (1) with signature: void (const std::string &)
static nv::LogSink* _bind_LogSink_sig1(lua_State* L) {
	// When reaching this call, we assume that the type checking is already done.
	int luatop = lua_gettop(L);
	size_t name_len = 0;
	const char* name_cstr = luatop>=1? lua_tolstring(L,1,&name_len) : nullptr;
	std::string name = luatop>=1? std::move(std::string(name_cstr, name_len)) : ""

	return new nv::LogSink(name);
}

In the function above I'm trying to create an instance of a LokSink object… yet, what I'm not taking into account here is that this class is abstract! So that will not work of course:

W:\Projects\NervSeed\sources\lua_bindings\wip_Core\src\luna\bind_nv_LogSink.cpp(140): error C2143: syntax error: missing ';' before 'return'
W:\Projects\NervSeed\sources\lua_bindings\wip_Core\src\luna\bind_nv_LogSink.cpp(140): error C2259: 'nv::LogSink': cannot instantiate abstract class
W:\Projects\NervSeed\sources\lua_bindings\wip_Core\src\luna\bind_nv_LogSink.cpp(140): note: due to following members:
W:\Projects\NervSeed\sources\lua_bindings\wip_Core\src\luna\bind_nv_LogSink.cpp(140): note: 'void nv::LogSink::output(int,std::string,std::string)': is abstract
w:\projects\nervseed\sources\nvcore\include\log\LogSink.h(55): note: see declaration of 'nv::LogSink::output'
bind_nv_LogSink_StringSet.cpp

But as I just said: enough for today! We'll get to this next time !

Happy hacking everyone!