Compare LuaRefs referencing Lua functions in C++ code

[dtroitskiy]

Here I am again, and this time it's more of a question rather than an issue.
Let's get straight to practical example with this one. So, there may be various needs to store Lua functions as LuaRefs on the C++ side, but I guess one of most common reasons is to establish some event subscription from scripts to events happening in C++.
A naive example may look like this:

static std::set<LuaRef> eventHandlers;

static void subscribe(LuaRef handler)
{
  if (handler.isFunction())
  {
    eventHandlers.insert(handler);
  }
}

static void trigger()
{
  for (auto handler : eventHandlers)
  {
    handler();
  }
}

int main()
{
  const auto L = getLuaState();
  registerMainThread(L);
  enableExceptions(L);

  getGlobalNamespace(L)
    .addFunction("subscribe", subscribe)
    .addFunction("trigger", trigger);

  loadScript("test.lua");

  trigger();

  return 0;
}

And then in Lua:

subscribe(function()
  print('Hello')
end)
subscribe(function()
  print('World')
end)

And here first function is added okay, but as soon as another one is attempted to be added, I get this error: attempt to compare two function values. As I understand, that's because internally std::set attempts to check function uniqueness compared to already added to that set and that gets to comparison of LuaRefs and somewhere inside LuaBridge code this is determined as incorrect operation.

Ability to compare functions is important in this concept, because as soon as you need to remove a function from event handlers collection, you need some way to identify this function. In Lua functions may be compared easily because I assume they're some kind of pointers, i.e. function variable can be printed as function: 0000024a923ade90 and thus same such values may be considered equal. So it's possible to store event handlers in Lua in form of table where function itself may even be a table key, so it can be like:

handlers = {}
local func = function()
  print('Hello')
end
-- subscribe
handlers[func] = func
-- unsubscribe
handlers[func] = nil

In C++, as I understand, we don't have a simple possibility to compare LuaRefs which are actually referencing function, is this correct?
In my code I already implemented a workaround where I have special Function class that wraps LuaRef, has unique ID per instance, and provides operator== that compares IDs of passed instances, so it can be used in std::set.
But this means in scripts I have to additionally wrap Lua function into that Function class I bind. So it's like:

local handler = Function(function()
  print('Hello')
end))
-- then I can pass `handler` to C++

But this means that wrapping everywhere in scripts. Maybe more elegant solution is possible?

[kunitoki]

LuaRef comparison operators will use lua_compare and will invoke metamethods. I'm not sure functions can be compared with that, and for sure they can't have a LuaRef::operator< which seems to be needed by std::set (it uses std::less<LuaRef> underneat). Maybe you get better results using std::unordered_set by also providing a KeyEqual comparator based on LuaRef::rawequal rather than LuaRef::operator== (which is selected by default by std:equal_to<LuaRef>).

I will see if i can make it work somehow.

[kunitoki]

If i replace your set with an unordered_set it works no problem. Is there a reason you need ordering ?

[dtroitskiy]

No, I don't need set specifically, to me it was just a convenient way to store unique functions without possibility to have duplicates, but this only works with that Function wrapper I wrote, which has that unique id field, which can be used in operator< as well. But yeah, I can use any other container, like unordered_set or list, I just need a way to compare functions to be able to find them in that container and delete when necessary.
So yeah, if you're able to compare LuaRef referencing functions on your side or you can provide more information about that KeyEqual / rawequal so I can use it in my code, that could already work well for me. In fact, if rawequal is able to compare functions in Lua style, i.e. using those addresses or whatever this is I described above, I mean function: 0000024a923ade90, that's good enough for me.

[dtroitskiy]

Hm, I just realized that if you wrote it works with unordered_set, this means comparison for equality works already, otherwise how unordered_set would check for item uniqueness, right? I just thought that error attempt to compare two function values appears in any attempt to compare functions, including equality. But if it's not, then I might have created an issue for nothing. Let me check that.

[kunitoki]

== works on functions, but < not

[kunitoki]

You can do this, but lua_pointer will return nullptr for trivial types:

struct LuaRefLess
{
    bool operator()(const luabridge::LuaRef& x, const luabridge::LuaRef& y) const
    {
        x.push();
        auto lhs = lua_topointer(x.state(), -1);

        y.push();
        auto rhs = lua_topointer(y.state(), -1);

        return lhs < rhs;
    }
};

TEST_F(LuaRefTests, SetComparatorOperatorLessThan)
{
    std::set<luabridge::LuaRef, LuaRefLess> eventHandlers;

    auto subscribe = [&eventHandlers](luabridge::LuaRef handler)
    {
        if (handler.isFunction())
            eventHandlers.insert(handler);
    };

    auto trigger = [&eventHandlers]
    {
        for (auto handler : eventHandlers)
            handler();
    };

    luabridge::getGlobalNamespace(L)
        .addFunction("subscribe", subscribe)
        .addFunction("trigger", trigger);

    runLua(R"(
        a = function()
            print('Hello')
        end
        b = a

        subscribe(a)
        subscribe(b)
        subscribe("wrong")

        subscribe(function()
            print('World')
        end)
    )");

    trigger();
}

[dtroitskiy]

Yep, I just checked and turns out there's no issue at all, looks like comparison for equality works already under the hood and thus I can delete LuaRef-functions from C++ container no problem. My bad, I misinterpreted the error, or rather haven't researched it enough. Additionally, I just checked in the intepreter and figured out this error comes from Lua itself, not from LuaBridge, as I assumed before, so yeah, this is how it is:

> f1 = function()end
> f2 = function()end
> f1 == f2
false
> f1 < f2
stdin:1: attempt to compare two function values
stack traceback:
        stdin:1: in main chunk
        [C]: in ?

Ok, thank you for your attention in any case, and I'm closing this issue then.

[kunitoki]

#65 < This could fix your issue and be more safe at the same time, as lua_compare might raise a lua error (and trigger the panic handler).

[dtroitskiy]

By looking at the changes I assume you compare pointer values there. I wanted to suggest the same, because initially I supposed pointers increase in value as function variables get initialized, but then I noticed it's not always the case, probably because of GC, so I started to think it's not that good idea as it seems that simple == is more reliable. But if you implemented it, I guess it can't hurt, because this should allow storing LuaRef in set, even in case if order of LuaRef being added to set won't match ordering inside of set because of that apparent address increment inconsistency.

[kunitoki]

I'm also not sure, but i also don't want that a call in c++ on luarefs will trigger the panic handler