LuaWrapper: Add support for std::variant
#16622
Conversation
Signed-off-by: Remi Gacogne <[email protected]>
|
|
||
| static PushedObject push(lua_State* state, const std::variant<TTypes...>& value) noexcept { | ||
| PushedObject obj{state, 0}; | ||
| std::visit([&](auto&& arg) { |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
It does, std::variant is a C++17 feature.
There was a problem hiding this comment.
Note that we added support for std::optional which is also a C++17 feature, so technically we already bumped it (oops :-/).
There was a problem hiding this comment.
oh, duh. I checked std::visit which is ++17, but that is of course obvious if variant itself is. Carry on :)
There was a problem hiding this comment.
Note that we added support for
std::optionalwhich is also a C++17 feature, so technically we already bumped it (oops :-/).
Right. No way back then. Do what feels good :)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>;
#endifThere was a problem hiding this comment.
ok, if it's that easy, why not :)
There was a problem hiding this comment.
unless the caller uses boost::optional :)
There was a problem hiding this comment.
I pushed a commit fixing C++11 compatibility for now.
| { | ||
| constexpr auto nbTypes = std::variant_size_v<ReturnType>; | ||
| if constexpr (I >= nbTypes) { | ||
| return boost::none; |
There was a problem hiding this comment.
you can take the variant out of the boost but you can't take the boost out of the luawrapper ;)
| using ReturnType = std::variant<TTypes...>; | ||
|
|
||
| private: | ||
| template<std::size_t I = 0> static boost::optional<ReturnType> variantRead(lua_State* state, int index) |
There was a problem hiding this comment.
I see we do support std::optional in luawrapper now, but you can't use it here?
There was a problem hiding this comment.
We could, but we need to convert all readers at the same time, so perhaps in a follow-up PR?
There was a problem hiding this comment.
right, I feared something like that
Pull Request Test Coverage Report for Build 20058059008Details
💛 - Coveralls |
Signed-off-by: Remi Gacogne <[email protected]>
Signed-off-by: Remi Gacogne <[email protected]>
Signed-off-by: Remi Gacogne <[email protected]>
Signed-off-by: Remi Gacogne <[email protected]>
5 participants
Short description
Checklist
I have: