Skip to content

Fix some bugs on rvariant for type-changing case #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saki7 merged 3 commits into from
Oct 5, 2026
+65 −13
Merged

Fix some bugs on rvariant for type-changing case #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3950b5
Add tests for emplace into the same type at another index
saki7 Oct 5, 2026
129cb93
Decide whether emplace reuses the contained object by its index
saki7 Oct 5, 2026
5d92999
Remove unreachable code
saki7 Oct 5, 2026
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
19 changes: 6 additions & 13 deletions include/iris/rvariant/rvariant.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,22 +452,15 @@ IRIS_RVARIANT_ALWAYS_THROWING_UNREACHABLE_BEGIN
this->template reset_construct_never_valueless<I>(std::forward<Args>(args)...);

} else {
this->raw_visit([&, this]<std::size_t old_i, class T_old_i>(std::in_place_index_t<old_i>, T_old_i& t_old_i)
noexcept(std::is_nothrow_constructible_v<T, Args...>)
{
this->raw_visit([&, this]<std::size_t old_i, class T_old_i>(std::in_place_index_t<old_i>, T_old_i& t_old_i) {
static_assert(!std::is_reference_v<T_old_i>);
static_assert(!std::is_const_v<T_old_i>);

if constexpr (old_i == std::variant_npos) {
(void)t_old_i;
this->template construct_on_valueless<I>(std::forward<Args>(args)...);

} else if constexpr (std::is_nothrow_constructible_v<T, Args...>) {
t_old_i.~T_old_i();
static_assert(std::is_nothrow_constructible_v<storage_type, std::in_place_index_t<old_i>, Args...>);
std::construct_at(&this->storage_, std::in_place_index<old_i>, std::forward<Args>(args)...);

} else if constexpr (std::is_same_v<T_old_i, T>) { // NOT type-changing
} else if constexpr (old_i == I) { // same alternative
if constexpr (
(sizeof(T) <= detail::never_valueless_trivial_size_limit && std::is_trivially_move_assignable_v<T>) ||
is_recursive_wrapper_v<T>
Expand Down Expand Up @@ -497,12 +490,12 @@ IRIS_RVARIANT_ALWAYS_THROWING_UNREACHABLE_BEGIN
static_assert(!never_valueless);
t_old_i.~T_old_i();
this->index_ = detail::variant_npos<sizeof...(Ts)>;
static_assert(!noexcept(std::construct_at(&this->storage(), std::in_place_index<old_i>, std::forward<Args>(args)...)));
std::construct_at(&this->storage_, std::in_place_index<old_i>, std::forward<Args>(args)...); // may throw
this->index_ = old_i;
static_assert(!noexcept(std::construct_at(&this->storage(), std::in_place_index<I>, std::forward<Args>(args)...)));
std::construct_at(&this->storage_, std::in_place_index<I>, std::forward<Args>(args)...); // may throw
this->index_ = I;
}

} else { // type-changing
} else { // another alternative, possibly of the same type
if constexpr (
(sizeof(T) <= detail::never_valueless_trivial_size_limit && std::is_trivially_move_constructible_v<T>) ||
is_recursive_wrapper_v<T>
Expand Down
59 changes: 59 additions & 0 deletions test/rvariant/rvariant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <exception>
#include <initializer_list>
#include <memory>
#include <string>
#include <type_traits>
#include <utility>
#include <variant>
Expand Down Expand Up @@ -1422,6 +1423,64 @@ TEST_CASE("emplace")
STATIC_REQUIRE(is_never_valueless<iris::rvariant<iris::recursive_wrapper<int>>>);

// ReSharper restore CppStaticAssertFailure

// `emplace<I>` must switch to the I-th alternative even when the current one has the same type
{
// NOLINTBEGIN(modernize-use-equals-default)
{
struct S
{
S(int value) noexcept(false) : value(value) {} // potentially-throwing
int value = 0;
};
iris::rvariant<S, S> v(std::in_place_index<0>, 1);
v.emplace<1>(2);
REQUIRE(v.index() == 1);
CHECK(iris::get<1>(v).value == 2);
v.emplace<0>(3);
REQUIRE(v.index() == 0);
CHECK(iris::get<0>(v).value == 3);
}
{
struct StrangeS
{
StrangeS(int value) noexcept(false) : value(value) {} // potentially-throwing
StrangeS(StrangeS&& other) noexcept : value(other.value) {} // not trivial
StrangeS(StrangeS const&) = default; // trivial
StrangeS& operator=(StrangeS&& other) noexcept { value = other.value; return *this; } // not trivial
StrangeS& operator=(StrangeS const&) = default; // trivial
int value = 0;
};
iris::rvariant<StrangeS, StrangeS> v(std::in_place_index<0>, 1);
v.emplace<1>(2);
REQUIRE(v.index() == 1);
CHECK(iris::get<1>(v).value == 2);
v.emplace<0>(3);
REQUIRE(v.index() == 0);
CHECK(iris::get<0>(v).value == 3);
}
// NOLINTEND(modernize-use-equals-default)
{
iris::rvariant<std::string, std::string> v(std::in_place_index<0>, "a");
v.emplace<1>("b");
REQUIRE(v.index() == 1);
CHECK(iris::get<1>(v) == "b");
v.emplace<0>("c");
REQUIRE(v.index() == 0);
CHECK(iris::get<0>(v) == "c");
}
{
iris::rvariant<iris::recursive_wrapper<int>, iris::recursive_wrapper<int>> v(std::in_place_index<0>, 1);
v.emplace<1>(2);
REQUIRE(v.index() == 1);
CHECK(iris::get<1>(v) == 2);
}
STATIC_CHECK([] {
iris::rvariant<std::string, std::string> v(std::in_place_index<0>, "a");
v.emplace<1>("b");
return v.index() == 1 && iris::get<1>(v) == "b";
}());
}
}


Expand Down
Loading