diff --git a/CHANGELOG.md b/CHANGELOG.md index 86d9ef30531..ea2f4001563 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/controllers/plugins/SolTypes.cpp b/src/controllers/plugins/SolTypes.cpp index 71e7dd4a33a..41d8062d1b2 100644 --- a/src/controllers/plugins/SolTypes.cpp +++ b/src/controllers/plugins/SolTypes.cpp @@ -40,7 +40,7 @@ void logError(Plugin *plugin, QStringView context, const QString &msg) // NOLINTBEGIN(readability-named-parameter) // QString bool sol_lua_check(sol::types, lua_State *L, int index, - std::function handler, + chatterino::FunctionRef handler, sol::stack::record &tracking) { return sol::stack::check(L, index, handler, tracking); @@ -60,7 +60,7 @@ int sol_lua_push(sol::types, lua_State *L, const QString &value) // QStringList bool sol_lua_check(sol::types, lua_State *L, int index, - std::function handler, + chatterino::FunctionRef handler, sol::stack::record &tracking) { return sol::stack::check(L, index, handler, tracking); @@ -92,7 +92,7 @@ int sol_lua_push(sol::types, lua_State *L, // QByteArray bool sol_lua_check(sol::types, lua_State *L, int index, - std::function handler, + chatterino::FunctionRef handler, sol::stack::record &tracking) { return sol::stack::check(L, index, handler, tracking); @@ -115,10 +115,11 @@ namespace chatterino::lua { // ThisPluginState -bool sol_lua_check(sol::types, - lua_State * /*L*/, int /* index*/, - std::function /* handler*/, - sol::stack::record & /*tracking*/) +bool sol_lua_check( + sol::types, lua_State * /*L*/, + int /* index*/, + chatterino::FunctionRef /* handler*/, + sol::stack::record & /*tracking*/) { return true; } diff --git a/src/controllers/plugins/SolTypes.hpp b/src/controllers/plugins/SolTypes.hpp index fdd6bedb6c6..d7a05c7b88c 100644 --- a/src/controllers/plugins/SolTypes.hpp +++ b/src/controllers/plugins/SolTypes.hpp @@ -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, lua_State *L, int index, \ - std::function handler, \ - sol::stack::record &tracking); \ - TYPE sol_lua_get(sol::types, lua_State *L, int index, \ - sol::stack::record &tracking); \ +# define SOL_STACK_FUNCTIONS(TYPE) \ + bool sol_lua_check( \ + sol::types, lua_State *L, int index, \ + chatterino::FunctionRef handler, \ + sol::stack::record &tracking); \ + TYPE sol_lua_get(sol::types, lua_State *L, int index, \ + sol::stack::record &tracking); \ int sol_lua_push(sol::types, lua_State *L, const TYPE &value); SOL_STACK_FUNCTIONS(chatterino::lua::ThisPluginState) diff --git a/src/util/FunctionRef.hpp b/src/util/FunctionRef.hpp new file mode 100644 index 00000000000..dc0435bfc5b --- /dev/null +++ b/src/util/FunctionRef.hpp @@ -0,0 +1,82 @@ +#pragma once + +#include +#include +#include +#include + +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 +class FunctionRef; + +template +class FunctionRef +{ +public: + FunctionRef() = default; + FunctionRef(std::nullptr_t) + { + } + + template + FunctionRef(Callable &&callable) // NOLINT + requires + // This is not the copy-constructor. + (!std::same_as, FunctionRef>) && + // Functor must be callable and return a suitable type. + (std::is_void_v || + std::convertible_to< + decltype(std::declval()(std::declval()...)), + Ret>) + : callback(callTrampoline>) + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + , callable(reinterpret_cast(&callable)) + { + } + + Ret operator()(Params... params) const + { + return this->callback(this->callable, std::forward(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 + static Ret callTrampoline(uintptr_t callable, Params... params) + { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + return (*reinterpret_cast(callable))( + std::forward(params)...); + } +}; + +} // namespace chatterino diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5f77e9b414d..ef88b4c5bf4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -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 diff --git a/tests/src/FunctionRef.cpp b/tests/src/FunctionRef.cpp new file mode 100644 index 00000000000..469a73995e3 --- /dev/null +++ b/tests/src/FunctionRef.cpp @@ -0,0 +1,171 @@ +#include "util/FunctionRef.hpp" + +#include "Test.hpp" + +#include + +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 fn, int i) +{ + return fn(i); +} + +} // namespace + +TEST(FunctionRef, lambda) +{ + static int i = 0; + auto empty = []() { + i++; + }; + + FunctionRef{empty}(); + ASSERT_EQ(i, 1); + + auto unique = std::make_unique(0); + int *up = unique.get(); + auto noncopyable = [p{std::move(unique)}]() { + *p += 1; + }; + FunctionRef{noncopyable}(); + ASSERT_EQ(*up, 1); + { + FunctionRef f{std::move(noncopyable)}; + f(); + ASSERT_EQ(*up, 2); + } + + auto identity = [](auto &&it) { + return std::forward(it); + }; + + ASSERT_EQ(FunctionRef{identity}(1), 1); + ASSERT_EQ( + FunctionRef{identity}(std::string{"foo"}), + "foo"); + + ASSERT_EQ(refCaller( + [](int i) { + return std::to_string(i); + }, + 42), + "42"); + + ASSERT_EQ(refCaller( + [p{std::make_unique(1)}](int i) { + return std::to_string(i + *p); + }, + 42), + "43"); +} + +TEST(FunctionRef, pointer) +{ + ASSERT_EQ(FunctionRef{myFunc}("foo"), 3); + ASSERT_EQ(FunctionRef{myFunc2}(2), "2"); + ASSERT_EQ(refCaller(myFunc2, 42), "42"); +} + +TEST(FunctionRef, stdFunction) +{ + int i = 0; + FunctionRef{std::function{[&] { + i++; + }}}(); + ASSERT_EQ(i, 1); + ASSERT_EQ(FunctionRef{std::function{myFunc}}("foo"), + 3); +} + +TEST(FunctionRef, operatorEq) +{ + int i = 0; + FunctionRef f1([] {}); + FunctionRef f2([&] { + i++; + }); + FunctionRef 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 f4(myFunc); + FunctionRef f5(myFunc); + FunctionRef 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 f0; + FunctionRef f1([] {}); + FunctionRef f2(myFunc); + + ASSERT_FALSE(f0); + ASSERT_TRUE(f1); + ASSERT_TRUE(f2); +} + +TEST(FunctionRef, copyCtor) +{ + FunctionRef f0; + auto f0Copy = f0; + ASSERT_EQ(f0, f0Copy); + ASSERT_FALSE(f0); + ASSERT_FALSE(f0Copy); + + int i = 0; + auto cb = [&] { + i++; + }; + FunctionRef f1(cb); + auto f1Copy = f1; + ASSERT_EQ(f1, f1Copy); + f1(); + ASSERT_EQ(i, 1); + f1Copy(); + ASSERT_EQ(i, 2); + + FunctionRef f2(myFunc); + auto f2Copy = f2; + ASSERT_EQ(f2, f2Copy); + ASSERT_EQ(f2("foobar"), f2Copy("foobar")); + + static_assert(std::is_trivially_copyable_v); + static_assert(std::is_trivially_copyable_v); + + static_assert(std::is_trivially_move_assignable_v); + static_assert(std::is_trivially_move_assignable_v); + + static_assert(std::is_trivially_move_constructible_v); + static_assert(std::is_trivially_move_constructible_v); +}