Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 67 additions & 2 deletions ext/luawrapper/include/LuaContext.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <list>
#include <map>
#include <memory>
#include <optional>
#include <random>
#include <set>
#include <stdexcept>
Expand All @@ -56,6 +55,11 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <boost/type_traits.hpp>
#include <lua.hpp>

#if __cplusplus >= 201703L
#include <variant>
#include <optional>
#endif /* C++17 */

// Set the older gcc define for tsan if clang or modern gcc tell us we have tsan.
#if defined(__has_feature)
#if __has_feature(thread_sanitizer)
Expand Down Expand Up @@ -2001,8 +2005,10 @@ struct LuaContext::FunctionArgumentsCounter<> {
// implementation of IsOptional
template<typename T>
struct LuaContext::IsOptional<boost::optional<T>> : public std::true_type {};
#if __cplusplus >= 201703L
template<typename T>
struct LuaContext::IsOptional<std::optional<T>> : public std::true_type {};
#endif /* C++ 17 */

// implementation of LuaFunctionCaller
template<typename TFunctionType>
Expand Down Expand Up @@ -2556,6 +2562,25 @@ struct LuaContext::Pusher<boost::variant<TTypes...>>
};
};

#if __cplusplus >= 201703L
// std::variant
template<typename... TTypes>
struct LuaContext::Pusher<std::variant<TTypes...>>
{
static const int minSize = PusherMinSize<TTypes...>::size;
static const int maxSize = PusherMaxSize<TTypes...>::size;

static PushedObject push(lua_State* state, const std::variant<TTypes...>& value) noexcept {
PushedObject obj{state, 0};
std::visit([&](auto&& arg) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this bump the luawrapper C++ requirement to C++17? Because in theory we try to stay in sync with upstream (which is me and I haven't spoken to any other users in years)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, std::variant is a C++17 feature.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we added support for std::optional which is also a C++17 feature, so technically we already bumped it (oops :-/).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, duh. I checked std::visit which is ++17, but that is of course obvious if variant itself is. Carry on :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we added support for std::optional which is also a C++17 feature, so technically we already bumped it (oops :-/).

Right. No way back then. Do what feels good :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good to keep in mind that this is possible if necessary, but I don't think we need to uglify the code with it now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might get away with something like:

template<class Type>
#if __cplusplus >= 201703L
using OptionalValue = std::optional<Type>;
#else
using OptionalValue = boost::optional<Type>;
#endif

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, if it's that easy, why not :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless the caller uses boost::optional :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit fixing C++11 compatibility for now.

using T = std::decay_t<decltype(arg)>;
obj = Pusher<T>::push(state, arg);
}, value);
return obj;
}
};
#endif /* C++17 */

// boost::optional
template<typename TType>
struct LuaContext::Pusher<boost::optional<TType>> {
Expand All @@ -2575,6 +2600,7 @@ struct LuaContext::Pusher<boost::optional<TType>> {
}
};

#if __cplusplus >= 201703L
// std::optional
template<typename TType>
struct LuaContext::Pusher<std::optional<TType>> {
Expand All @@ -2593,6 +2619,7 @@ struct LuaContext::Pusher<std::optional<TType>> {
}
}
};
#endif /* C++17 */

// tuple
template<typename... TTypes>
Expand Down Expand Up @@ -2943,6 +2970,7 @@ struct LuaContext::Reader<boost::optional<TType>>
}
};

#if __cplusplus >= 201703L
// NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks)
template<typename TType>
struct LuaContext::Reader<std::optional<TType>>
Expand All @@ -2960,8 +2988,9 @@ struct LuaContext::Reader<std::optional<TType>>
}
};
// NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks)
#endif /* C++ 17 */

// variant
// boost::variant
template<typename... TTypes>
struct LuaContext::Reader<boost::variant<TTypes...>>
{
Expand Down Expand Up @@ -3008,6 +3037,42 @@ struct LuaContext::Reader<boost::variant<TTypes...>>
}
};

#if __cplusplus >= 201703L
// std::variant
template<typename... TTypes>
struct LuaContext::Reader<std::variant<TTypes...>>
{
using ReturnType = std::variant<TTypes...>;

private:
template<std::size_t I = 0> static boost::optional<ReturnType> variantRead(lua_State* state, int index)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we do support std::optional in luawrapper now, but you can't use it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but we need to convert all readers at the same time, so perhaps in a follow-up PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I feared something like that

{
constexpr auto nbTypes = std::variant_size_v<ReturnType>;
if constexpr (I >= nbTypes) {
return boost::none;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can take the variant out of the boost but you can't take the boost out of the luawrapper ;)

}
using InitialType = std::variant_alternative_t<I, ReturnType>;
using DecayedType = std::decay_t<InitialType>;
if (const auto val = Reader<DecayedType>::read(state, index)) {
// we are using the initial, non-decayed type so that a
// cv-qualifiers are not ignored
return ReturnType{std::in_place_type<InitialType>, *val};
}
if constexpr (I < (nbTypes - 1)) {
return variantRead<I + 1>(state, index);
}
return boost::none;
}

public:
static auto read(lua_State* state, int index)
-> boost::optional<ReturnType>
{
return variantRead(state, index);
}
};
#endif /* C++ 17 */

// reading a tuple
// tuple have an additional argument for their functions, that is the maximum size to read
// if maxSize is smaller than the tuple size, then the remaining parameters will be left to default value
Expand Down
60 changes: 60 additions & 0 deletions pdns/test-luawrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,64 @@ BOOST_AUTO_TEST_CASE(test_std_optional)
}
}

BOOST_AUTO_TEST_CASE(test_boost_variant)
{
using MyVariantType = boost::variant<int, const std::string, std::string, std::string*>;

LuaContext context;
context.writeFunction("testVariant", [](MyVariantType incoming) -> MyVariantType {
return incoming;
});

{
auto result = context.executeCode<MyVariantType>("return testVariant(1)");
const auto* content = boost::get<int>(&result);
BOOST_REQUIRE(content);
BOOST_CHECK_EQUAL(*content, 1);
}

{
auto result = context.executeCode<MyVariantType>("return testVariant('foo')");
const auto* content = boost::get<const std::string>(&result);
BOOST_REQUIRE(content);
BOOST_CHECK_EQUAL(*content, "foo");
}

{
auto func = [&]() {
context.executeCode<MyVariantType>("return testVariant(nil)");
};
BOOST_CHECK_THROW(func(), LuaContext::ExecutionErrorException);
}
}

BOOST_AUTO_TEST_CASE(test_std_variant)
{
using MyVariantType = std::variant<int, const std::string, std::string, std::string*>;

LuaContext context;
context.writeFunction("testVariant", [](MyVariantType incoming) -> MyVariantType {
return incoming;
});

{
const auto result = context.executeCode<MyVariantType>("return testVariant(1)");
BOOST_REQUIRE(std::holds_alternative<int>(result));
BOOST_CHECK_EQUAL(std::get<int>(result), 1);
}

{
const auto result = context.executeCode<MyVariantType>("return testVariant('foo')");
BOOST_REQUIRE(std::holds_alternative<const std::string>(result));
BOOST_CHECK_EQUAL(std::get<const std::string>(result), "foo");
}

{
auto func = [&]() {
context.executeCode<MyVariantType>("return testVariant(nil)");
};
BOOST_CHECK_THROW(func(), LuaContext::ExecutionErrorException);
}
}

BOOST_AUTO_TEST_SUITE_END()