fix: take handler for sol_lua_check as function reference (#6393)

* fix: take handler for `sol_lua_check` as function reference

* changelog

* fix: includes

* uintptr type

* fix: ub and eq
This commit is contained in:
nerix
2025-08-16 21:21:04 +02:00
committed by GitHub
parent d8aa2b2846
commit 0f11a9050a
6 changed files with 271 additions and 13 deletions
+1
View File
@@ -103,6 +103,7 @@
- Dev: Update vcpkg baseline. (#6359)
- Dev: Added an explicit `frozen` flag to `Message`. (#6367)
- Dev: Stop sending `JOIN`/`PART` commands for channels starting with `/`. (#6376)
- Dev: Error handlers for Sol check functions now take a function reference over an owning type. (#6393)
## 2.5.3
+8 -7
View File
@@ -40,7 +40,7 @@ void logError(Plugin *plugin, QStringView context, const QString &msg)
// NOLINTBEGIN(readability-named-parameter)
// QString
bool sol_lua_check(sol::types<QString>, lua_State *L, int index,
std::function<sol::check_handler_type> handler,
chatterino::FunctionRef<sol::check_handler_type> handler,
sol::stack::record &tracking)
{
return sol::stack::check<const char *>(L, index, handler, tracking);
@@ -60,7 +60,7 @@ int sol_lua_push(sol::types<QString>, lua_State *L, const QString &value)
// QStringList
bool sol_lua_check(sol::types<QStringList>, lua_State *L, int index,
std::function<sol::check_handler_type> handler,
chatterino::FunctionRef<sol::check_handler_type> handler,
sol::stack::record &tracking)
{
return sol::stack::check<sol::table>(L, index, handler, tracking);
@@ -92,7 +92,7 @@ int sol_lua_push(sol::types<QStringList>, lua_State *L,
// QByteArray
bool sol_lua_check(sol::types<QByteArray>, lua_State *L, int index,
std::function<sol::check_handler_type> handler,
chatterino::FunctionRef<sol::check_handler_type> handler,
sol::stack::record &tracking)
{
return sol::stack::check<const char *>(L, index, handler, tracking);
@@ -115,10 +115,11 @@ namespace chatterino::lua {
// ThisPluginState
bool sol_lua_check(sol::types<chatterino::lua::ThisPluginState>,
lua_State * /*L*/, int /* index*/,
std::function<sol::check_handler_type> /* handler*/,
sol::stack::record & /*tracking*/)
bool sol_lua_check(
sol::types<chatterino::lua::ThisPluginState>, lua_State * /*L*/,
int /* index*/,
chatterino::FunctionRef<sol::check_handler_type> /* handler*/,
sol::stack::record & /*tracking*/)
{
return true;
}
+8 -6
View File
@@ -1,5 +1,6 @@
#pragma once
#ifdef CHATTERINO_HAVE_PLUGINS
# include "util/FunctionRef.hpp"
# include "util/QMagicEnum.hpp"
# include "util/TypeName.hpp"
@@ -174,12 +175,13 @@ void loggedVoidCall(const auto &fn, QStringView context, Plugin *plugin,
}
// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
# define SOL_STACK_FUNCTIONS(TYPE) \
bool sol_lua_check(sol::types<TYPE>, lua_State *L, int index, \
std::function<sol::check_handler_type> handler, \
sol::stack::record &tracking); \
TYPE sol_lua_get(sol::types<TYPE>, lua_State *L, int index, \
sol::stack::record &tracking); \
# define SOL_STACK_FUNCTIONS(TYPE) \
bool sol_lua_check( \
sol::types<TYPE>, lua_State *L, int index, \
chatterino::FunctionRef<sol::check_handler_type> handler, \
sol::stack::record &tracking); \
TYPE sol_lua_get(sol::types<TYPE>, lua_State *L, int index, \
sol::stack::record &tracking); \
int sol_lua_push(sol::types<TYPE>, lua_State *L, const TYPE &value);
SOL_STACK_FUNCTIONS(chatterino::lua::ThisPluginState)
+82
View File
@@ -0,0 +1,82 @@
#pragma once
#include <concepts>
#include <cstdint>
#include <type_traits>
#include <utility>
namespace chatterino {
/// A non-owning, type-erased reference to a callable. Callables can be lambdas,
/// functions, or std::functions.
///
/// It is intended to be used for functions taking a callback that is called
/// immediately (such as filter/map functions). Since this doesn't own the
/// callable, it's not safe to store.
///
/// This is based on llvm::function_ref (updated for C++ 20).
template <typename Fn>
class FunctionRef;
template <typename Ret, typename... Params>
class FunctionRef<Ret(Params...)>
{
public:
FunctionRef() = default;
FunctionRef(std::nullptr_t)
{
}
template <typename Callable>
FunctionRef(Callable &&callable) // NOLINT
requires
// This is not the copy-constructor.
(!std::same_as<std::remove_cvref_t<Callable>, FunctionRef>) &&
// Functor must be callable and return a suitable type.
(std::is_void_v<Ret> ||
std::convertible_to<
decltype(std::declval<Callable>()(std::declval<Params>()...)),
Ret>)
: callback(callTrampoline<std::remove_reference_t<Callable>>)
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
, callable(reinterpret_cast<uintptr_t>(&callable))
{
}
Ret operator()(Params... params) const
{
return this->callback(this->callable, std::forward<Params>(params)...);
}
explicit operator bool() const
{
return this->callback != nullptr;
}
bool operator==(const FunctionRef &other) const
{
return this->callback == other.callback &&
this->callable == other.callable;
}
bool operator!=(const FunctionRef &other) const = default;
private:
// same signature as callTrampoline
using Callback = Ret(uintptr_t, Params...);
/// Pointer to the call trampoline that, given the callable, calls the target function.
Callback *callback = nullptr;
/// Pointer to the actual function
uintptr_t callable = 0;
template <typename Callable>
static Ret callTrampoline(uintptr_t callable, Params... params)
{
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
return (*reinterpret_cast<Callable *>(callable))(
std::forward<Params>(params)...);
}
};
} // namespace chatterino
+1
View File
@@ -58,6 +58,7 @@ set(test_SOURCES
${CMAKE_CURRENT_LIST_DIR}/src/ImageUploader.cpp
${CMAKE_CURRENT_LIST_DIR}/src/TwitchChannel.cpp
${CMAKE_CURRENT_LIST_DIR}/src/TwitchUserColor.cpp
${CMAKE_CURRENT_LIST_DIR}/src/FunctionRef.cpp
${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.cpp
${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.hpp
+171
View File
@@ -0,0 +1,171 @@
#include "util/FunctionRef.hpp"
#include "Test.hpp"
#include <memory>
using namespace chatterino;
namespace {
size_t myFunc(const std::string &s)
{
return s.size();
}
std::string myFunc2(int i)
{
return std::to_string(i);
}
std::string refCaller(FunctionRef<std::string(int)> fn, int i)
{
return fn(i);
}
} // namespace
TEST(FunctionRef, lambda)
{
static int i = 0;
auto empty = []() {
i++;
};
FunctionRef<void()>{empty}();
ASSERT_EQ(i, 1);
auto unique = std::make_unique<int>(0);
int *up = unique.get();
auto noncopyable = [p{std::move(unique)}]() {
*p += 1;
};
FunctionRef<void()>{noncopyable}();
ASSERT_EQ(*up, 1);
{
FunctionRef<void()> f{std::move(noncopyable)};
f();
ASSERT_EQ(*up, 2);
}
auto identity = [](auto &&it) {
return std::forward<decltype(it)>(it);
};
ASSERT_EQ(FunctionRef<int(int)>{identity}(1), 1);
ASSERT_EQ(
FunctionRef<std::string(std::string)>{identity}(std::string{"foo"}),
"foo");
ASSERT_EQ(refCaller(
[](int i) {
return std::to_string(i);
},
42),
"42");
ASSERT_EQ(refCaller(
[p{std::make_unique<int>(1)}](int i) {
return std::to_string(i + *p);
},
42),
"43");
}
TEST(FunctionRef, pointer)
{
ASSERT_EQ(FunctionRef<int(std::string)>{myFunc}("foo"), 3);
ASSERT_EQ(FunctionRef<std::string(int)>{myFunc2}(2), "2");
ASSERT_EQ(refCaller(myFunc2, 42), "42");
}
TEST(FunctionRef, stdFunction)
{
int i = 0;
FunctionRef<void()>{std::function{[&] {
i++;
}}}();
ASSERT_EQ(i, 1);
ASSERT_EQ(FunctionRef<size_t(std::string)>{std::function{myFunc}}("foo"),
3);
}
TEST(FunctionRef, operatorEq)
{
int i = 0;
FunctionRef<void()> f1([] {});
FunctionRef<void()> f2([&] {
i++;
});
FunctionRef<void()> f3([=]() mutable {
i++;
});
ASSERT_EQ(f1, f1);
ASSERT_NE(f1, f2);
ASSERT_NE(f1, f3);
ASSERT_EQ(f2, f2);
ASSERT_NE(f2, f1);
ASSERT_NE(f2, f3);
ASSERT_EQ(f3, f3);
ASSERT_NE(f3, f1);
ASSERT_NE(f3, f2);
FunctionRef<size_t(std::string)> f4(myFunc);
FunctionRef<size_t(std::string)> f5(myFunc);
FunctionRef<size_t(std::string)> f6([](std::string /*s*/) { // NOLINT
return 0;
});
ASSERT_EQ(f4, f4);
ASSERT_EQ(f4, f5);
ASSERT_EQ(f5, f4);
ASSERT_NE(f4, f6);
ASSERT_NE(f6, f4);
ASSERT_EQ(f6, f6);
}
TEST(FunctionRef, operatorBool)
{
FunctionRef<void()> f0;
FunctionRef<void()> f1([] {});
FunctionRef<size_t(std::string)> f2(myFunc);
ASSERT_FALSE(f0);
ASSERT_TRUE(f1);
ASSERT_TRUE(f2);
}
TEST(FunctionRef, copyCtor)
{
FunctionRef<void()> f0;
auto f0Copy = f0;
ASSERT_EQ(f0, f0Copy);
ASSERT_FALSE(f0);
ASSERT_FALSE(f0Copy);
int i = 0;
auto cb = [&] {
i++;
};
FunctionRef<void()> f1(cb);
auto f1Copy = f1;
ASSERT_EQ(f1, f1Copy);
f1();
ASSERT_EQ(i, 1);
f1Copy();
ASSERT_EQ(i, 2);
FunctionRef<size_t(std::string)> f2(myFunc);
auto f2Copy = f2;
ASSERT_EQ(f2, f2Copy);
ASSERT_EQ(f2("foobar"), f2Copy("foobar"));
static_assert(std::is_trivially_copyable_v<decltype(f0)>);
static_assert(std::is_trivially_copyable_v<decltype(f2)>);
static_assert(std::is_trivially_move_assignable_v<decltype(f0)>);
static_assert(std::is_trivially_move_assignable_v<decltype(f2)>);
static_assert(std::is_trivially_move_constructible_v<decltype(f0)>);
static_assert(std::is_trivially_move_constructible_v<decltype(f2)>);
}