From c5fd55e0907d62caa9f08c52bbe0ea427c379754 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Thu, 1 Oct 2026 10:44:39 +0200 Subject: [PATCH 1/4] Fix ownership of the content of Variant Copy and move of Variants holding Array2D, LabeledArray or string array values copied the storage byte by byte, so copies shared the same heap buffers, and the destructor never destroyed these objects, leaking them. Assignments never released the previous content, move assignment of strings and arrays duplicated the buffer and dropped the original one, and move assignment of string arrays corrupted the source vector. --- Framework/Core/include/Framework/Variant.h | 42 ++-- Framework/Core/src/Variant.cxx | 232 ++++++++++++--------- Framework/Core/test/test_Variants.cxx | 168 ++++++++++++++- 3 files changed, 319 insertions(+), 123 deletions(-) diff --git a/Framework/Core/include/Framework/Variant.h b/Framework/Core/include/Framework/Variant.h index 7121c5fad0669..3611fad438e0a 100644 --- a/Framework/Core/include/Framework/Variant.h +++ b/Framework/Core/include/Framework/Variant.h @@ -13,6 +13,7 @@ #include "Framework/RuntimeError.h" #include "Framework/Array2D.h" +#include "Framework/Traits.h" #include #include #include @@ -261,9 +262,9 @@ struct variant_helper { new (reinterpret_cast(store)) T{}; *(reinterpret_cast(store)) = value; } - static void set(void* store, T values, size_t size) + static void set(void* store, std::remove_pointer_t const* values, size_t size) { - *reinterpret_cast(store) = reinterpret_cast(std::memcpy(std::malloc(size * sizeof(std::remove_pointer_t)), reinterpret_cast(values), size * sizeof(std::remove_pointer_t))); + *reinterpret_cast(store) = reinterpret_cast(std::memcpy(std::malloc(size * sizeof(std::remove_pointer_t)), reinterpret_cast(values), size * sizeof(std::remove_pointer_t))); } static T get(const void* store) { return *(reinterpret_cast(store)); } @@ -317,9 +318,10 @@ struct variant_helper { class Variant { public: - Variant(VariantType type = VariantType::Unknown) : mType{type} {} + Variant(VariantType type = VariantType::Unknown); template + requires(!is_specialization_v) Variant(T value) : mType{variant_trait_v} { variant_helper::set(&mStore, value); @@ -331,17 +333,23 @@ class Variant variant_helper::set(&mStore, values, mSize); } + // A Variant owns what it stores: the content of vectors is copied template - Variant(std::vector& values) : mType{variant_trait_v}, mSize{values.size()} + Variant(std::vector const& values) : mType{variant_trait_v}, mSize{values.size()} { variant_helper::set(&mStore, values.data(), mSize); } - Variant(std::vector& values) : mType{VariantType::ArrayString}, mSize{values.size()} + Variant(std::vector const& values) : mType{VariantType::ArrayString}, mSize{values.size()} { variant_helper>::set(&mStore, values); } + // A temporary vector cannot hand over its buffer to a Variant, + // so creating one only to copy it is an error + template + Variant(std::vector&&) = delete; + template Variant(std::initializer_list) { @@ -355,6 +363,8 @@ class Variant ~Variant(); Variant& operator=(const Variant& other); Variant& operator=(Variant&& other) noexcept; + // Assignment from a temporary vector has to be allowed because it is used, but copies + // to make sure Variant owns its content template Variant& operator=(std::vector&& other) noexcept { @@ -381,31 +391,25 @@ class Variant return variant_helper::get(&mStore); } + // The setters replace the current content, releasing it first. template void set(T value) { - return variant_helper::set(&mStore, value); + *this = Variant(value); } template void set(T value, size_t size) { - mSize = size; - return variant_helper::set(&mStore, value, mSize); - } - - template - void set(std::vector& values) - requires(std::is_pod_v) - { - return variant_helper::set(&mStore, values.data(), values.size()); + *this = Variant(value, size); } + /// FIXME: set for vector of strings is not used anywhere, why? template void set(std::vector& values) - requires(std::is_same_v) + requires(std::is_pod_v || std::is_same_v) { - return variant_helper::set(&mStore, values); + *this = Variant(values); } [[nodiscard]] VariantType type() const { return mType; } @@ -414,6 +418,10 @@ class Variant private: friend std::ostream& operator<<(std::ostream& oss, Variant const& val); + // Helpers to manage the store depending on the actual content + void copyStore(Variant const& other); + void moveStore(Variant& other) noexcept; + void destroyStore() noexcept; using storage_t = std::aligned_union<8, int, int8_t, int16_t, int64_t, uint8_t, uint16_t, uint32_t, uint64_t, const char*, float, double, bool, diff --git a/Framework/Core/src/Variant.cxx b/Framework/Core/src/Variant.cxx index e54a973bd4413..f780a1a952f1b 100644 --- a/Framework/Core/src/Variant.cxx +++ b/Framework/Core/src/Variant.cxx @@ -11,7 +11,10 @@ #include "Framework/Variant.h" #include "Framework/VariantPropertyTreeHelpers.h" #include "Framework/VariantJSONHelpers.h" +#include +#include #include +#include #include namespace o2::framework @@ -84,145 +87,166 @@ std::string Variant::asString() const return ss.str(); } -Variant::Variant(const Variant& other) : mType(other.mType) +namespace { - // In case this is an array we need to duplicate it to avoid - // double deletion. - switch (mType) { - case VariantType::String: - mSize = other.mSize; - variant_helper::set(&mStore, other.get()); - return; - case VariantType::ArrayInt: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; - case VariantType::ArrayFloat: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; - case VariantType::ArrayDouble: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; - case VariantType::ArrayBool: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; +/// Helper visitor for Variant +template +bool visitStoredObject(VariantType type, F&& f) +{ + switch (type) { case VariantType::ArrayString: - mSize = other.mSize; - variant_helper>::set(&mStore, other.get>()); - return; + f.template operator()>(); + return true; + case VariantType::Array2DInt: + f.template operator()>(); + return true; + case VariantType::Array2DFloat: + f.template operator()>(); + return true; + case VariantType::Array2DDouble: + f.template operator()>(); + return true; + case VariantType::LabeledArrayInt: + f.template operator()>(); + return true; + case VariantType::LabeledArrayFloat: + f.template operator()>(); + return true; + case VariantType::LabeledArrayDouble: + f.template operator()>(); + return true; + case VariantType::LabeledArrayString: + f.template operator()>(); + return true; default: - mStore = other.mStore; - mSize = other.mSize; + return false; } } -Variant::Variant(Variant&& other) noexcept : mType(other.mType) +/// Types for which the storage keeps a pointer to a manually allocated buffer +bool holdsMallocedPointer(VariantType type) { - mStore = other.mStore; - mSize = other.mSize; - switch (mType) { + switch (type) { case VariantType::String: case VariantType::ArrayInt: case VariantType::ArrayFloat: case VariantType::ArrayDouble: case VariantType::ArrayBool: - case VariantType::ArrayString: - *reinterpret_cast(&(other.mStore)) = nullptr; - return; + return true; default: - return; + return false; } } -Variant::~Variant() +template +T* copyBuffer(T const* values, size_t size) +{ + if (values == nullptr) { + return nullptr; + } + return reinterpret_cast(std::memcpy(std::malloc(size * sizeof(T)), values, size * sizeof(T))); +} +} // namespace + +Variant::Variant(VariantType type) : mType{type} +{ + // Make sure that destroying a Variant created without a value is always safe + // by creating a default stored object upfront + if (!visitStoredObject(mType, [this]() { new (&mStore) T{}; })) { + std::memset(&mStore, 0, sizeof(mStore)); + } +} + +void Variant::copyStore(Variant const& other) { - // In case we allocated an array, we - // should delete it. + // Proper objects are simply copied + if (visitStoredObject(mType, [this, &other]() { new (&mStore) T(*reinterpret_cast(&other.mStore)); })) { + return; + } + // Manually allocated buffers have to be managed switch (mType) { - case VariantType::String: + case VariantType::String: { + auto const* value = *reinterpret_cast(&other.mStore); + *reinterpret_cast(&mStore) = value != nullptr ? strdup(value) : nullptr; + return; + } case VariantType::ArrayInt: + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); + return; case VariantType::ArrayFloat: + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); + return; case VariantType::ArrayDouble: - case VariantType::ArrayBool: { - if (reinterpret_cast(&mStore) != nullptr) { - free(*reinterpret_cast(&mStore)); - } + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); return; - } - case VariantType::ArrayString: { - // Allocated with placement new. Nothing to delete. + case VariantType::ArrayBool: + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); return; - } default: - return; + // Trivially copyable content + mStore = other.mStore; + } +} + +void Variant::moveStore(Variant& other) noexcept +{ + // Correct move for objects, leaving proper "moved from" state + if (visitStoredObject(mType, [this, &other]() { new (&mStore) T(std::move(*reinterpret_cast(&other.mStore))); })) { + return; + } + mStore = other.mStore; + // Buffers have to change their owner + if (holdsMallocedPointer(mType)) { + *reinterpret_cast(&other.mStore) = nullptr; + } +} + +void Variant::destroyStore() noexcept +{ + // destroy objects + if (visitStoredObject(mType, [this]() { std::destroy_at(reinterpret_cast(&mStore)); })) { + return; + } + // deallocate buffers + if (holdsMallocedPointer(mType)) { + free(*reinterpret_cast(&mStore)); } } +Variant::Variant(const Variant& other) : mType(other.mType), mSize(other.mSize) +{ + copyStore(other); +} + +Variant::Variant(Variant&& other) noexcept : mType(other.mType), mSize(other.mSize) +{ + moveStore(other); +} + +Variant::~Variant() +{ + destroyStore(); +} + Variant& Variant::operator=(const Variant& other) { - mSize = other.mSize; - mType = other.mType; - switch (mType) { - case VariantType::String: - variant_helper::set(&mStore, other.get()); - return *this; - case VariantType::ArrayInt: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayFloat: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayDouble: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayBool: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayString: - variant_helper>::set(&mStore, other.get>()); - return *this; - default: - mStore = other.mStore; - return *this; + if (this != &other) { + // Copy first, so that a throwing copy leaves this Variant untouched + Variant copy(other); + *this = std::move(copy); } + return *this; } Variant& Variant::operator=(Variant&& other) noexcept { - mSize = other.mSize; - mType = other.mType; - switch (mType) { - case VariantType::String: - variant_helper::set(&mStore, other.get()); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayInt: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayFloat: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayDouble: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayBool: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayString: - variant_helper>::set(&mStore, other.get>()); - *reinterpret_cast**>(&(other.mStore)) = nullptr; - return *this; - default: - mStore = other.mStore; - return *this; + if (this != &other) { + destroyStore(); + mType = other.mType; + mSize = other.mSize; + moveStore(other); } + return *this; } std::pair, std::vector> extractLabels(boost::property_tree::ptree const& tree) diff --git a/Framework/Core/test/test_Variants.cxx b/Framework/Core/test/test_Variants.cxx index c47a54eb68e62..244dd3c9359c5 100644 --- a/Framework/Core/test/test_Variants.cxx +++ b/Framework/Core/test/test_Variants.cxx @@ -287,6 +287,88 @@ TEST_CASE("Array2DAssignmentTest") REQUIRE(msc(1, 0) == "two"); } +TEST_CASE("VariantLifecycleTest") +{ + float m[2][3] = {{1, 2, 3}, {4, 5, 6}}; + LabeledArray laf{&m[0][0], 2, 3, {"r1", "r2"}, {"c1", "c2", "c3"}}; + std::vector vs{"s1", "s2", "s3"}; + auto checkLabeled = [&](Variant const& v) { + REQUIRE(v.type() == VariantType::LabeledArrayFloat); + auto la = v.get>(); + REQUIRE(la.rows() == 2); + REQUIRE(la.cols() == 3); + REQUIRE(la.get("r2", "c3") == 6); + REQUIRE(la.getLabelsRows() == std::vector{"r1", "r2"}); + }; + + Variant vl(laf); + // copies are independent and survive the destruction of the original + auto* copy = new Variant(vl); + Variant moved(std::move(*copy)); + delete copy; + checkLabeled(vl); + checkLabeled(moved); + + // assignment across types releases the previous content + Variant vstr("a string"); + vstr = vl; + checkLabeled(vstr); + vstr = Variant("back to a string"); + REQUIRE(vstr.type() == VariantType::String); + REQUIRE(std::string(vstr.get()) == "back to a string"); + vstr = std::move(moved); + checkLabeled(vstr); + Variant vvs(vs); + vstr = vvs; + REQUIRE(vstr.get>() == vs); + + // self assignment keeps the content + auto& self = vl; + vl = self; + checkLabeled(vl); + vl = std::move(self); + checkLabeled(vl); + + // moving a string array leaves a valid moved-from Variant behind + Variant vvsMoved(std::move(vvs)); + REQUIRE(vvsMoved.get>() == vs); + vvs = vvsMoved; + REQUIRE(vvs.get>() == vs); + + // reallocation of a container moves the Variants around + std::vector collection; + for (auto i = 0; i < 20; ++i) { + collection.emplace_back(laf); + collection.emplace_back(Array2D{&m[0][0], 2, 3}); + collection.emplace_back(vs); + collection.emplace_back("a string"); + } + std::vector collectionCopy = collection; + collection.clear(); + for (auto i = 0U; i < collectionCopy.size(); i += 4) { + checkLabeled(collectionCopy[i]); + REQUIRE(collectionCopy[i + 1].get>()(1, 2) == 6); + REQUIRE(collectionCopy[i + 2].get>() == vs); + REQUIRE(std::string(collectionCopy[i + 3].get()) == "a string"); + } + + // a Variant created with only a type can be copied and destroyed + Variant typed(VariantType::LabeledArrayFloat); + Variant typedCopy(typed); + REQUIRE(typedCopy.get>().rows() == 0); + Variant typedString(VariantType::String); + Variant typedStringCopy(typedString); + REQUIRE(typedStringCopy.get() == nullptr); + + // set replaces the content and the type + Variant vset(1); + vset.set(laf); + checkLabeled(vset); + vset.set(vs); + REQUIRE(vset.type() == VariantType::ArrayString); + REQUIRE(vset.get>() == vs); +} + TEST_CASE("LabeledArrayTest") { float m[3][4] = {{0.1, 0.2, 0.3, 0.4}, {0.5, 0.6, 0.7, 0.8}, {0.9, 1.0, 1.1, 1.2}}; @@ -311,11 +393,14 @@ TEST_CASE("LabeledArrayTest") TEST_CASE("VariantTreeConversionsTest") { std::vector vstrings{"0 1", "0 2", "0 3"}; - Variant vvstr(std::move(vstrings)); + Variant vvstr(vstrings); auto tree = vectorToBranch(vvstr.get(), vvstr.size()); - auto v = Variant(vectorFromBranch(tree)); + auto fromTree = vectorFromBranch(tree); + auto v = Variant(fromTree); + REQUIRE(vvstr.size() == vstrings.size()); + REQUIRE(v.size() == vstrings.size()); for (auto i = 0U; i < vvstr.size(); ++i) { REQUIRE(vvstr.get()[i] == v.get()[i]); } @@ -410,3 +495,82 @@ TEST_CASE("VariantThrowing") REQUIRE(error.what == std::string("Variant::get: Mismatch between types 4 0.")); } } + +// A Variant can be created from a vector only by copying its content +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +// assignment from a temporary vector copies it +static_assert(std::is_assignable_v>); +static_assert(std::is_assignable_v>); +static_assert(std::is_constructible_v&>); +static_assert(std::is_constructible_v const&>); +static_assert(std::is_constructible_v const&>); +static_assert(std::is_assignable_v const&>); + +namespace +{ +template +void checkVectorCopy(VariantType type, std::vector source) +{ + auto check = [&](Variant const& v, std::vector const& from) { + REQUIRE(v.type() == type); + REQUIRE(v.size() == from.size()); + auto const* stored = v.get(); + REQUIRE(stored != from.data()); + for (auto i = 0U; i < from.size(); ++i) { + REQUIRE(stored[i] == from[i]); + } + }; + std::vector const constSource = source; + Variant fromConst(constSource); + check(fromConst, constSource); + + Variant fromMutable(source); + check(fromMutable, source); + // the Variant owns a copy, unaffected by later changes of the source + auto const original = source; + source[0] = source.back(); + source.push_back(source[0]); + check(fromMutable, original); +} +} // namespace + +TEST_CASE("VariantFromVectorTest") +{ + checkVectorCopy(VariantType::ArrayInt, {1, 2, 3, 4}); + checkVectorCopy(VariantType::ArrayFloat, {0.5f, 1.5f, 2.5f}); + checkVectorCopy(VariantType::ArrayDouble, {1e-3, 1e3}); + checkVectorCopy(VariantType::ArrayString, {"a", "bb", "ccc"}); + + // assignment from a vector copies as well + std::vector vi{7, 8, 9}; + Variant v(1); + v = vi; + REQUIRE(v.type() == VariantType::ArrayInt); + REQUIRE(v.size() == 3); + REQUIRE(v.get() != vi.data()); + REQUIRE(v.get()[2] == 9); + // also from a temporary + auto makeStrings = []() { return std::vector{"x", "y"}; }; + v = makeStrings(); + REQUIRE(v.type() == VariantType::ArrayString); + REQUIRE(v.size() == 2); + REQUIRE(v.get()[1] == "y"); + v = std::vector{1.5, 2.5, 3.5}; + REQUIRE(v.type() == VariantType::ArrayDouble); + REQUIRE(v.size() == 3); + REQUIRE(v.get()[2] == 3.5); + + // a Variant created from a const vector round-trips through JSON + std::vector const vd{0.25, 0.5, 0.75}; + Variant vdv(vd); + std::stringstream is(vdv.asString()); + auto read = VariantJSONHelpers::read(is); + REQUIRE(read.size() == vd.size()); + for (auto i = 0U; i < vd.size(); ++i) { + REQUIRE(read.get()[i] == vd[i]); + } +} From c6c0800fb87d81361c5828885ba45caf4379bfe2 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Mon, 12 Oct 2026 13:00:04 +0200 Subject: [PATCH 2/4] nitpicks --- Framework/Core/include/Framework/Variant.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Framework/Core/include/Framework/Variant.h b/Framework/Core/include/Framework/Variant.h index 3611fad438e0a..5d590f671e2d9 100644 --- a/Framework/Core/include/Framework/Variant.h +++ b/Framework/Core/include/Framework/Variant.h @@ -322,7 +322,7 @@ class Variant template requires(!is_specialization_v) - Variant(T value) : mType{variant_trait_v} + Variant(T value) : mType{variant_trait_v}, mSize{1} { variant_helper::set(&mStore, value); } @@ -345,8 +345,7 @@ class Variant variant_helper>::set(&mStore, values); } - // A temporary vector cannot hand over its buffer to a Variant, - // so creating one only to copy it is an error + // Prevent constructing from a temporary vector to avoid double allocation template Variant(std::vector&&) = delete; @@ -366,7 +365,7 @@ class Variant // Assignment from a temporary vector has to be allowed because it is used, but copies // to make sure Variant owns its content template - Variant& operator=(std::vector&& other) noexcept + Variant& operator=(std::vector&& other) //not noexcept because it allocates { *this = Variant(other); return *this; @@ -430,7 +429,7 @@ class Variant LabeledArray, LabeledArray, LabeledArray, LabeledArray>::type; storage_t mStore; VariantType mType = VariantType::Unknown; - size_t mSize = 1; + size_t mSize = 0; }; inline Variant emptyDict() { return Variant(VariantType::Dict); } From b8cc6c842e069acaeed317beb08aedebbe507bc0 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Mon, 12 Oct 2026 13:13:50 +0200 Subject: [PATCH 3/4] reset plain array size to 0 --- Framework/Core/include/Framework/Variant.h | 5 ++--- Framework/Core/src/Variant.cxx | 11 +++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Framework/Core/include/Framework/Variant.h b/Framework/Core/include/Framework/Variant.h index 5d590f671e2d9..286a3969355a2 100644 --- a/Framework/Core/include/Framework/Variant.h +++ b/Framework/Core/include/Framework/Variant.h @@ -322,7 +322,7 @@ class Variant template requires(!is_specialization_v) - Variant(T value) : mType{variant_trait_v}, mSize{1} + Variant(T value) : mType{variant_trait_v} { variant_helper::set(&mStore, value); } @@ -403,7 +403,6 @@ class Variant *this = Variant(value, size); } - /// FIXME: set for vector of strings is not used anywhere, why? template void set(std::vector& values) requires(std::is_pod_v || std::is_same_v) @@ -429,7 +428,7 @@ class Variant LabeledArray, LabeledArray, LabeledArray, LabeledArray>::type; storage_t mStore; VariantType mType = VariantType::Unknown; - size_t mSize = 0; + size_t mSize = 1; }; inline Variant emptyDict() { return Variant(VariantType::Dict); } diff --git a/Framework/Core/src/Variant.cxx b/Framework/Core/src/Variant.cxx index f780a1a952f1b..35d20f85ba919 100644 --- a/Framework/Core/src/Variant.cxx +++ b/Framework/Core/src/Variant.cxx @@ -154,6 +154,17 @@ Variant::Variant(VariantType type) : mType{type} // by creating a default stored object upfront if (!visitStoredObject(mType, [this]() { new (&mStore) T{}; })) { std::memset(&mStore, 0, sizeof(mStore)); + switch (type) { + case VariantType::ArrayBool: + case VariantType::ArrayInt: + case VariantType::ArrayFloat: + case VariantType::ArrayDouble: + case VariantType::ArrayString: + mSize = 0; // empty plain array has size 0 + break; + default: + break; + } } } From 5062301f79c241406feca689ecedb6f5993c9dde Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Mon, 12 Oct 2026 13:15:10 +0200 Subject: [PATCH 4/4] format --- Framework/Core/include/Framework/Variant.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Framework/Core/include/Framework/Variant.h b/Framework/Core/include/Framework/Variant.h index 286a3969355a2..c56c0c01ef9fc 100644 --- a/Framework/Core/include/Framework/Variant.h +++ b/Framework/Core/include/Framework/Variant.h @@ -365,7 +365,7 @@ class Variant // Assignment from a temporary vector has to be allowed because it is used, but copies // to make sure Variant owns its content template - Variant& operator=(std::vector&& other) //not noexcept because it allocates + Variant& operator=(std::vector&& other) // not noexcept because it allocates { *this = Variant(other); return *this;