AK: Fix Variant construction from lvalue references

Fixes #7371 and appends its test cases.
This commit is contained in:
Ali Mohammad Pur 2021-05-22 09:46:40 +04:30 committed by Andreas Kling
parent 3f350c3b65
commit 6b4d7b6c19
Notes: sideshowbarker 2024-07-18 17:34:47 +09:00
2 changed files with 64 additions and 21 deletions

View File

@ -106,25 +106,18 @@ template<typename T, typename Base>
struct VariantConstructors {
VariantConstructors(T&& t)
{
internal_cast().template set<T>(forward<T>(t), VariantNoClearTag {});
internal_cast().clear_without_destruction();
internal_cast().set(move(t), VariantNoClearTag {});
}
VariantConstructors(const T& t)
{
internal_cast().clear_without_destruction();
internal_cast().set(t, VariantNoClearTag {});
}
VariantConstructors() { }
Base& operator=(const T& value)
{
Base variant { value };
internal_cast() = move(variant);
return internal_cast();
}
Base& operator=(T&& value)
{
Base variant { move(value) };
internal_cast() = move(variant);
return internal_cast();
}
private:
[[nodiscard]] Base& internal_cast()
{
@ -213,6 +206,7 @@ public:
Variant(const Variant& old)
: Detail::MergeAndDeduplicatePacks<Detail::VariantConstructors<Ts, Variant<Ts...>>...>()
, m_data {}
, m_index(old.m_index)
{
Helper::copy_(old.m_index, old.m_data, m_data);
@ -224,6 +218,7 @@ public:
// but it will still contain the "moved-from" state of the object it previously contained.
Variant(Variant&& old)
: Detail::MergeAndDeduplicatePacks<Detail::VariantConstructors<Ts, Variant<Ts...>>...>()
, m_data {}
, m_index(old.m_index)
{
Helper::move_(old.m_index, old.m_data, m_data);
@ -250,20 +245,18 @@ public:
using Detail::MergeAndDeduplicatePacks<Detail::VariantConstructors<Ts, Variant<Ts...>>...>::MergeAndDeduplicatePacks;
template<typename T>
void set(T&& t) requires(index_of<T>() != invalid_index)
template<typename T, typename StrippedT = RemoveCV<RemoveReference<T>>>
void set(T&& t) requires(index_of<StrippedT>() != invalid_index)
{
using StrippedT = RemoveCV<RemoveReference<T>>;
constexpr auto new_index = index_of<StrippedT>();
Helper::delete_(m_index, m_data);
new (m_data) StrippedT(forward<T>(t));
m_index = new_index;
}
template<typename T>
void set(T&& t, Detail::VariantNoClearTag)
template<typename T, typename StrippedT = RemoveCV<RemoveReference<T>>>
void set(T&& t, Detail::VariantNoClearTag) requires(index_of<StrippedT>() != invalid_index)
{
using StrippedT = RemoveCV<RemoveReference<T>>;
constexpr auto new_index = index_of<StrippedT>();
new (m_data) StrippedT(forward<T>(t));
m_index = new_index;
@ -355,12 +348,21 @@ private:
using Helper = Detail::Variant<IndexType, 0, Ts...>;
using VisitHelper = Detail::VisitImpl<IndexType, Ts...>;
template<typename T_, typename U_>
friend struct Detail::VariantConstructors;
explicit Variant(IndexType index, Detail::VariantConstructTag)
: Detail::MergeAndDeduplicatePacks<Detail::VariantConstructors<Ts, Variant<Ts...>>...>()
, m_index(index)
{
}
void clear_without_destruction()
{
__builtin_memset(m_data, 0, data_size);
m_index = invalid_index;
}
template<typename... Fs>
struct Visitor : Fs... {
Visitor(Fs&&... args)

View File

@ -172,3 +172,44 @@ TEST_CASE(return_values_by_reference)
EXPECT_EQ(ref->ref_count(), 1u);
EXPECT_EQ(value->ref_count(), 1u);
}
struct HoldsInt {
int i;
};
struct HoldsFloat {
float f;
};
TEST_CASE(copy_assign)
{
{
Variant<int, String, float> the_value { 42.0f };
VERIFY(the_value.has<float>());
EXPECT_EQ(the_value.get<float>(), 42.0f);
int twelve = 12;
the_value = twelve;
VERIFY(the_value.has<int>());
EXPECT_EQ(the_value.get<int>(), 12);
the_value = String("Hello, world!");
VERIFY(the_value.has<String>());
EXPECT_EQ(the_value.get<String>(), "Hello, world!");
}
{
Variant<HoldsInt, String, HoldsFloat> the_value { HoldsFloat { 42.0f } };
VERIFY(the_value.has<HoldsFloat>());
EXPECT_EQ(the_value.get<HoldsFloat>().f, 42.0f);
HoldsInt twelve { 12 };
the_value = twelve;
VERIFY(the_value.has<HoldsInt>());
EXPECT_EQ(the_value.get<HoldsInt>().i, 12);
the_value = String("Hello, world!");
VERIFY(the_value.has<String>());
EXPECT_EQ(the_value.get<String>(), "Hello, world!");
}
}