From d8fce1c1de67252da722d7a53839054f17622d85 Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Wed, 25 Sep 2024 19:26:04 +0200 Subject: [PATCH 1/4] Added test to deal with lvalues in variants --- tests/json/test_rfl_variant_visit_lvalues.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tests/json/test_rfl_variant_visit_lvalues.cpp diff --git a/tests/json/test_rfl_variant_visit_lvalues.cpp b/tests/json/test_rfl_variant_visit_lvalues.cpp new file mode 100644 index 00000000..1361e0c7 --- /dev/null +++ b/tests/json/test_rfl_variant_visit_lvalues.cpp @@ -0,0 +1,28 @@ +#include +#include +#include +#include +#include +#include + +#include "write_and_read.hpp" + +namespace test_rfl_variant_visit_lvalues { + +struct Rectangle { + double height; + double width; +}; + +struct Square { + double width; +}; + +using Shapes = rfl::Variant>; + +TEST(json, test_rfl_variant) { + const Shapes r = Rectangle{.height = 10, .width = 5}; + const auto get_width = [](const auto _s) -> double& { return _s.width; }; + EXPECT_EQ(std::visit(get_width, r), 5); +} +} // namespace test_rfl_variant_visit_lvalues From 69e685e0544f4b97544ef84f20ad15128647f9df Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Sat, 28 Sep 2024 00:41:12 +0200 Subject: [PATCH 2/4] Made sure the return typo can be a reference; #187 --- include/rfl/Variant.hpp | 150 +++++++++++++----- tests/json/test_rfl_variant_visit_lvalues.cpp | 8 +- 2 files changed, 113 insertions(+), 45 deletions(-) diff --git a/include/rfl/Variant.hpp b/include/rfl/Variant.hpp index 6178f541..1a9dfc64 100644 --- a/include/rfl/Variant.hpp +++ b/include/rfl/Variant.hpp @@ -33,6 +33,12 @@ class Variant { std::numeric_limits::max(), std::uint8_t, std::uint16_t>; + using FirstAlternative = internal::nth_element_t<0, AlternativeTypes...>; + + template + using result_t = std::remove_cv_t< + std::invoke_result_t, FirstAlternative&>>; + static constexpr IndexType size_ = sizeof...(AlternativeTypes); template @@ -146,14 +152,17 @@ class Variant { } template - auto visit(F& _f) { - using FirstAlternative = internal::nth_element_t<0, AlternativeTypes...>; - using ResultType = std::remove_cvref_t< - std::invoke_result_t, FirstAlternative&>>; + result_t visit(F& _f) { + using ResultType = result_t; if constexpr (std::is_same_v) { bool visited = false; do_visit_no_result(_f, &visited, std::make_integer_sequence()); + } else if constexpr (std::is_reference_v) { + std::remove_reference_t* res = nullptr; + do_visit_with_reference(_f, &res, + std::make_integer_sequence()); + return *res; } else { auto res = std::optional(); do_visit_with_result(_f, &res, @@ -163,14 +172,17 @@ class Variant { } template - auto visit(F& _f) const { - using FirstAlternative = internal::nth_element_t<0, AlternativeTypes...>; - using ResultType = std::remove_cvref_t< - std::invoke_result_t, FirstAlternative&>>; + result_t visit(F& _f) const { + using ResultType = result_t; if constexpr (std::is_same_v) { bool visited = false; do_visit_no_result(_f, &visited, std::make_integer_sequence()); + } else if constexpr (std::is_reference_v) { + std::remove_reference_t* res = nullptr; + do_visit_with_reference(_f, &res, + std::make_integer_sequence()); + return *res; } else { auto res = std::optional(); do_visit_with_result(_f, &res, @@ -180,14 +192,17 @@ class Variant { } template - auto visit(const F& _f) { - using FirstAlternative = internal::nth_element_t<0, AlternativeTypes...>; - using ResultType = std::remove_cvref_t< - std::invoke_result_t, FirstAlternative&>>; + result_t visit(const F& _f) { + using ResultType = std::remove_reference_t>; if constexpr (std::is_same_v) { bool visited = false; do_visit_no_result(_f, &visited, std::make_integer_sequence()); + } else if constexpr (std::is_reference_v) { + std::remove_reference_t* res = nullptr; + do_visit_with_reference(_f, &res, + std::make_integer_sequence()); + return *res; } else { auto res = std::optional(); do_visit_with_result(_f, &res, @@ -197,14 +212,17 @@ class Variant { } template - auto visit(const F& _f) const { - using FirstAlternative = internal::nth_element_t<0, AlternativeTypes...>; - using ResultType = std::remove_cvref_t< - std::invoke_result_t, FirstAlternative&>>; + result_t visit(const F& _f) const { + using ResultType = result_t; if constexpr (std::is_same_v) { bool visited = false; do_visit_no_result(_f, &visited, std::make_integer_sequence()); + } else if constexpr (std::is_reference_v) { + std::remove_reference_t* res = nullptr; + do_visit_with_reference(_f, &res, + std::make_integer_sequence()); + return *res; } else { auto res = std::optional(); do_visit_with_result(_f, &res, @@ -264,32 +282,6 @@ class Variant { (visit_one(_f, _visited, Index<_is>{}), ...); } - template - void do_visit_with_result(F& _f, std::optional* _res, - std::integer_sequence) { - auto visit_one = [this](const F& _f, - std::optional* _res, - Index<_i>) { - if (!*_res && index_ == _i) { - *_res = _f(get_alternative<_i>()); - } - }; - (visit_one(_f, _res, Index<_is>{}), ...); - } - - template - void do_visit_with_result(F& _f, std::optional* _res, - std::integer_sequence) const { - auto visit_one = [this](const F& _f, - std::optional* _res, - Index<_i>) { - if (!*_res && index_ == _i) { - *_res = _f(get_alternative<_i>()); - } - }; - (visit_one(_f, _res, Index<_is>{}), ...); - } - template void do_visit_no_result(const F& _f, bool* _visited, std::integer_sequence) { @@ -316,6 +308,32 @@ class Variant { (visit_one(_f, _visited, Index<_is>{}), ...); } + template + void do_visit_with_result(F& _f, std::optional* _res, + std::integer_sequence) { + auto visit_one = [this](const F& _f, + std::optional* _res, + Index<_i>) { + if (!*_res && index_ == _i) { + *_res = _f(get_alternative<_i>()); + } + }; + (visit_one(_f, _res, Index<_is>{}), ...); + } + + template + void do_visit_with_result(F& _f, std::optional* _res, + std::integer_sequence) const { + auto visit_one = [this](const F& _f, + std::optional* _res, + Index<_i>) { + if (!*_res && index_ == _i) { + *_res = _f(get_alternative<_i>()); + } + }; + (visit_one(_f, _res, Index<_is>{}), ...); + } + template void do_visit_with_result(const F& _f, std::optional* _res, std::integer_sequence) { @@ -342,6 +360,54 @@ class Variant { (visit_one(_f, _res, Index<_is>{}), ...); } + template + void do_visit_with_reference(F& _f, ResultType** _res, + std::integer_sequence) { + const auto visit_one = [this](const F& _f, ResultType** _res, + Index<_i>) { + if (!*_res && index_ == _i) { + *_res = &_f(get_alternative<_i>()); + } + }; + (visit_one(_f, _res, Index<_is>{}), ...); + } + + template + void do_visit_with_reference(F& _f, ResultType** _res, + std::integer_sequence) const { + const auto visit_one = [this](const F& _f, ResultType** _res, + Index<_i>) { + if (!*_res && index_ == _i) { + *_res = &_f(get_alternative<_i>()); + } + }; + (visit_one(_f, _res, Index<_is>{}), ...); + } + + template + void do_visit_with_reference(const F& _f, ResultType** _res, + std::integer_sequence) { + const auto visit_one = [this](const F& _f, ResultType** _res, + Index<_i>) { + if (!*_res && index_ == _i) { + *_res = &_f(get_alternative<_i>()); + } + }; + (visit_one(_f, _res, Index<_is>{}), ...); + } + + template + void do_visit_with_reference(const F& _f, ResultType** _res, + std::integer_sequence) const { + const auto visit_one = [this](const F& _f, ResultType** _res, + Index<_i>) { + if (!*_res && index_ == _i) { + *_res = &_f(get_alternative<_i>()); + } + }; + (visit_one(_f, _res, Index<_is>{}), ...); + } + template auto& get_alternative() noexcept { using CurrentType = internal::nth_element_t<_i, AlternativeTypes...>; diff --git a/tests/json/test_rfl_variant_visit_lvalues.cpp b/tests/json/test_rfl_variant_visit_lvalues.cpp index 1361e0c7..3eecc815 100644 --- a/tests/json/test_rfl_variant_visit_lvalues.cpp +++ b/tests/json/test_rfl_variant_visit_lvalues.cpp @@ -18,11 +18,13 @@ struct Square { double width; }; -using Shapes = rfl::Variant>; +using Shapes = rfl::Variant; TEST(json, test_rfl_variant) { const Shapes r = Rectangle{.height = 10, .width = 5}; - const auto get_width = [](const auto _s) -> double& { return _s.width; }; - EXPECT_EQ(std::visit(get_width, r), 5); + const auto get_width = [](const auto& _s) -> const double& { + return _s.width; + }; + EXPECT_EQ(rfl::visit(get_width, r), 5); } } // namespace test_rfl_variant_visit_lvalues From 4da96b8f247749c589587417a674b1c2bb2839ce Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Sat, 28 Sep 2024 11:14:31 +0200 Subject: [PATCH 3/4] Use integral_constant for the Index --- include/rfl/Variant.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rfl/Variant.hpp b/include/rfl/Variant.hpp index 1a9dfc64..8a89a933 100644 --- a/include/rfl/Variant.hpp +++ b/include/rfl/Variant.hpp @@ -42,7 +42,7 @@ class Variant { static constexpr IndexType size_ = sizeof...(AlternativeTypes); template - struct Index {}; + using Index = std::integral_constant; public: Variant() { From 8f107c238ff080fd6ae6c90daa14bfc5e9e83b4c Mon Sep 17 00:00:00 2001 From: Patrick Urbanke Date: Sat, 28 Sep 2024 11:48:49 +0200 Subject: [PATCH 4/4] Added support for non-copyable types, adresses #184 --- include/rfl/Variant.hpp | 12 ++-- include/rfl/internal/variant/result_t.hpp | 16 +++++ include/rfl/visit.hpp | 63 ++++++++++++------- .../json/test_rfl_variant_visit_move_only.cpp | 32 ++++++++++ 4 files changed, 93 insertions(+), 30 deletions(-) create mode 100644 include/rfl/internal/variant/result_t.hpp create mode 100644 tests/json/test_rfl_variant_visit_move_only.cpp diff --git a/include/rfl/Variant.hpp b/include/rfl/Variant.hpp index 8a89a933..b6041823 100644 --- a/include/rfl/Variant.hpp +++ b/include/rfl/Variant.hpp @@ -14,6 +14,7 @@ #include "internal/nth_element_t.hpp" #include "internal/variant/find_max_size.hpp" #include "internal/variant/is_alternative_type.hpp" +#include "internal/variant/result_t.hpp" namespace rfl { @@ -24,8 +25,6 @@ class Variant { static constexpr unsigned long num_bytes_ = max_size_wrapper_.size_; - using LargestType = typename decltype(max_size_wrapper_)::Type; - using DataType = std::array; using IndexType = @@ -33,13 +32,10 @@ class Variant { std::numeric_limits::max(), std::uint8_t, std::uint16_t>; - using FirstAlternative = internal::nth_element_t<0, AlternativeTypes...>; + static constexpr IndexType size_ = sizeof...(AlternativeTypes); template - using result_t = std::remove_cv_t< - std::invoke_result_t, FirstAlternative&>>; - - static constexpr IndexType size_ = sizeof...(AlternativeTypes); + using result_t = internal::variant::result_t; template using Index = std::integral_constant; @@ -442,7 +438,7 @@ class Variant { IndexType index_; /// The underlying data, can be any of the underlying types. - alignas(LargestType) DataType data_; + alignas(AlternativeTypes...) DataType data_; }; template diff --git a/include/rfl/internal/variant/result_t.hpp b/include/rfl/internal/variant/result_t.hpp new file mode 100644 index 00000000..a906c5af --- /dev/null +++ b/include/rfl/internal/variant/result_t.hpp @@ -0,0 +1,16 @@ +#ifndef RFL_INTERNAL_VARIANT_RESULT_T_HPP_ +#define RFL_INTERNAL_VARIANT_RESULT_T_HPP_ + +#include + +#include "../nth_element_t.hpp" + +namespace rfl::internal::variant { + +template +using result_t = std::remove_cv_t, internal::nth_element_t<0, AlternativeTypes...>&>>; + +} // namespace rfl::internal::variant + +#endif diff --git a/include/rfl/visit.hpp b/include/rfl/visit.hpp index a2f78618..2d98067d 100644 --- a/include/rfl/visit.hpp +++ b/include/rfl/visit.hpp @@ -8,6 +8,7 @@ #include "internal/StringLiteral.hpp" #include "internal/VisitTree.hpp" #include "internal/VisitorWrapper.hpp" +#include "internal/variant/result_t.hpp" namespace rfl { @@ -22,66 +23,84 @@ inline auto visit(const Visitor& _visitor, const Literal<_fields...> _literal, } template -inline auto visit(F& _f, Variant& _v) { +inline internal::variant::result_t visit( + F& _f, Variant& _v) { return _v.visit(_f); } template -inline auto visit(F& _f, Variant&& _v) { +inline internal::variant::result_t visit( + F& _f, Variant&& _v) { return _v.visit(_f); } template -inline auto visit(F& _f, const Variant& _v) { +inline internal::variant::result_t visit( + F& _f, const Variant& _v) { return _v.visit(_f); } template -inline auto visit(const F& _f, Variant& _v) { +inline internal::variant::result_t visit( + const F& _f, Variant& _v) { return _v.visit(_f); } template -inline auto visit(const F& _f, Variant&& _v) { +inline internal::variant::result_t visit( + const F& _f, Variant&& _v) { return _v.visit(_f); } template -inline auto visit(const F& _f, const Variant& _v) { +inline internal::variant::result_t visit( + const F& _f, const Variant& _v) { return _v.visit(_f); } -template -inline auto visit(F& _f, TaggedUnion<_discriminator, Args...>& _tagged_union) { +template +inline internal::variant::result_t visit( + F& _f, TaggedUnion<_discriminator, AlternativeTypes...>& _tagged_union) { return _tagged_union.variant().visit(_f); } -template -inline auto visit(F& _f, TaggedUnion<_discriminator, Args...>&& _tagged_union) { +template +inline internal::variant::result_t visit( + F& _f, TaggedUnion<_discriminator, AlternativeTypes...>&& _tagged_union) { return _tagged_union.variant().visit(_f); } -template -inline auto visit(F& _f, - const TaggedUnion<_discriminator, Args...>& _tagged_union) { +template +inline internal::variant::result_t visit( + F& _f, + const TaggedUnion<_discriminator, AlternativeTypes...>& _tagged_union) { return _tagged_union.variant().visit(_f); } -template -inline auto visit(const F& _f, - TaggedUnion<_discriminator, Args...>& _tagged_union) { +template +inline internal::variant::result_t visit( + const F& _f, + TaggedUnion<_discriminator, AlternativeTypes...>& _tagged_union) { return _tagged_union.variant().visit(_f); } -template -inline auto visit(const F& _f, - TaggedUnion<_discriminator, Args...>&& _tagged_union) { +template +inline internal::variant::result_t visit( + const F& _f, + TaggedUnion<_discriminator, AlternativeTypes...>&& _tagged_union) { return _tagged_union.variant().visit(_f); } -template -inline auto visit(const F& _f, - const TaggedUnion<_discriminator, Args...>& _tagged_union) { +template +inline internal::variant::result_t visit( + const F& _f, + const TaggedUnion<_discriminator, AlternativeTypes...>& _tagged_union) { return _tagged_union.variant().visit(_f); } diff --git a/tests/json/test_rfl_variant_visit_move_only.cpp b/tests/json/test_rfl_variant_visit_move_only.cpp new file mode 100644 index 00000000..5418ddec --- /dev/null +++ b/tests/json/test_rfl_variant_visit_move_only.cpp @@ -0,0 +1,32 @@ +#include +#include +#include +#include +#include +#include +#include + +#include "write_and_read.hpp" + +namespace test_rfl_variant_visit_move_only { + +struct Rectangle { + double height; + std::unique_ptr width; +}; + +struct Square { + std::unique_ptr width; +}; + +using Shapes = rfl::Variant; + +TEST(json, test_rfl_variant_visit_move_only) { + const Shapes r = + Rectangle{.height = 10, .width = std::make_unique(5.0)}; + const auto get_width = [](const auto& _s) -> const std::unique_ptr& { + return _s.width; + }; + EXPECT_EQ(*rfl::visit(get_width, r), 5.0); +} +} // namespace test_rfl_variant_visit_move_only