Skip to content

DPL: cleanup Variant copying for ArrayString #14864

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
ktf merged 1 commit into from
Nov 28, 2025
Merged

DPL: cleanup Variant copying for ArrayString #14864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 23 additions & 2 deletions Framework/Core/include/Framework/Variant.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,24 @@ struct variant_helper<std::vector<std::string>> {
// Allocates a new store and copies into it.
static void set(void* store, std::vector<std::string> value)
{
new (reinterpret_cast<std::vector<std::string>*>(store)) std::vector<std::string>{};
*(reinterpret_cast<std::vector<std::string>*>(store)) = value;
auto ptr = reinterpret_cast<std::vector<std::string>*>(store);
new (ptr) std::vector<std::string>{value};
}

static std::vector<std::string> const& get(const void* store) { return *(reinterpret_cast<std::vector<std::string> const*>(store)); }
};

template <>
struct variant_helper<std::string*> {
static void set(void* store, std::string* values, size_t size)
{
auto ptr = reinterpret_cast<std::vector<std::string>*>(store);
new (ptr) std::vector<std::string>{values, values + size};
}

static std::string const* get(const void* store) { return (*(reinterpret_cast<std::vector<std::string> const*>(store))).data(); }
};

template <>
struct variant_helper<const char*> {
static const char* get(const void* store) { return *reinterpret_cast<const char* const*>(store); }
Expand Down Expand Up @@ -360,6 +371,16 @@ class Variant
return variant_helper<T>::get(&mStore);
}

template <typename T>
[[nodiscard]] std::string const* get() const
requires(std::same_as<std::string*, T>)
{
if (mType != VariantType::ArrayString) {
throw runtime_error_f("Variant::get: Mismatch between types %d %d.", mType, VariantType::ArrayString);
}
return variant_helper<T>::get(&mStore);
}

template <typename T>
void set(T value)
{
Expand Down
14 changes: 7 additions & 7 deletions Framework/Core/include/Framework/VariantPropertyTreeHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ boost::property_tree::ptree basicVectorToBranch(std::vector<T>&& values)
}

template <typename T>
boost::property_tree::ptree vectorToBranch(T* values, size_t size)
boost::property_tree::ptree vectorToBranch(T const* values, size_t size)
{
boost::property_tree::ptree branch;
branch.put_child("values", basicVectorToBranch(values, size));
Expand Down Expand Up @@ -150,17 +150,17 @@ extern template boost::property_tree::ptree o2::framework::basicVectorToBranch(f
extern template boost::property_tree::ptree o2::framework::basicVectorToBranch(int*, size_t);
extern template boost::property_tree::ptree o2::framework::basicVectorToBranch(double*, size_t);
extern template boost::property_tree::ptree o2::framework::basicVectorToBranch(bool*, size_t);
extern template boost::property_tree::ptree o2::framework::basicVectorToBranch(std::basic_string<char>*, size_t);
extern template boost::property_tree::ptree o2::framework::basicVectorToBranch(std::basic_string<char> const*, size_t);

extern template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<float>&& values);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<int>&& values);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<double>&& values);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<std::string>&& values);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(float*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(int*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(double*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(bool*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(std::basic_string<char>*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(float const*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(int const*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(double const*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(bool const*, size_t);
extern template boost::property_tree::ptree o2::framework::vectorToBranch(std::basic_string<char> const*, size_t);

extern template boost::property_tree::ptree o2::framework::labeledArrayToBranch(o2::framework::LabeledArray<float>&& array);
extern template boost::property_tree::ptree o2::framework::labeledArrayToBranch(o2::framework::LabeledArray<int>&& array);
Expand Down
83 changes: 39 additions & 44 deletions Framework/Core/src/Variant.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,29 +89,29 @@ Variant::Variant(const Variant& other) : mType(other.mType)
// In case this is an array we need to duplicate it to avoid
// double deletion.
switch (mType) {
case variant_trait_v<const char*>:
case VariantType::String:
mSize = other.mSize;
variant_helper<const char*>::set(&mStore, other.get<const char*>());
return;
case variant_trait_v<int*>:
case VariantType::ArrayInt:
mSize = other.mSize;
variant_helper<int*>::set(&mStore, other.get<int*>(), mSize);
return;
case variant_trait_v<float*>:
case VariantType::ArrayFloat:
mSize = other.mSize;
variant_helper<float*>::set(&mStore, other.get<float*>(), mSize);
return;
case variant_trait_v<double*>:
case VariantType::ArrayDouble:
mSize = other.mSize;
variant_helper<double*>::set(&mStore, other.get<double*>(), mSize);
return;
case variant_trait_v<bool*>:
case VariantType::ArrayBool:
mSize = other.mSize;
variant_helper<bool*>::set(&mStore, other.get<bool*>(), mSize);
return;
case variant_trait_v<std::string*>:
case VariantType::ArrayString:
mSize = other.mSize;
variant_helper<std::string*>::set(&mStore, other.get<std::string*>(), mSize);
variant_helper<std::vector<std::string>>::set(&mStore, other.get<std::vector<std::string>>());
return;
default:
mStore = other.mStore;
Expand All @@ -124,23 +124,14 @@ Variant::Variant(Variant&& other) noexcept : mType(other.mType)
mStore = other.mStore;
mSize = other.mSize;
switch (mType) {
case variant_trait_v<const char*>:
*reinterpret_cast<char**>(&(other.mStore)) = nullptr;
return;
case variant_trait_v<int*>:
*reinterpret_cast<int**>(&(other.mStore)) = nullptr;
return;
case variant_trait_v<float*>:
*reinterpret_cast<float**>(&(other.mStore)) = nullptr;
return;
case variant_trait_v<double*>:
*reinterpret_cast<double**>(&(other.mStore)) = nullptr;
return;
case variant_trait_v<bool*>:
*reinterpret_cast<bool**>(&(other.mStore)) = nullptr;
case VariantType::String:
case VariantType::ArrayInt:
case VariantType::ArrayFloat:
case VariantType::ArrayDouble:
case VariantType::ArrayBool:
case VariantType::ArrayString:
*reinterpret_cast<void**>(&(other.mStore)) = nullptr;
return;
case variant_trait_v<std::string*>:
*reinterpret_cast<std::string**>(&(other.mStore)) = nullptr;
default:
return;
}
Expand All @@ -151,16 +142,20 @@ Variant::~Variant()
// In case we allocated an array, we
// should delete it.
switch (mType) {
case variant_trait_v<const char*>:
case variant_trait_v<int*>:
case variant_trait_v<float*>:
case variant_trait_v<double*>:
case variant_trait_v<bool*>:
case variant_trait_v<std::string*>:
case VariantType::String:
case VariantType::ArrayInt:
case VariantType::ArrayFloat:
case VariantType::ArrayDouble:
case VariantType::ArrayBool: {
if (reinterpret_cast<void**>(&mStore) != nullptr) {
free(*reinterpret_cast<void**>(&mStore));
}
return;
}
case VariantType::ArrayString: {
// Allocated with placement new. Nothing to delete.
return;
}
default:
return;
}
Expand All @@ -171,23 +166,23 @@ Variant& Variant::operator=(const Variant& other)
mSize = other.mSize;
mType = other.mType;
switch (mType) {
case variant_trait_v<const char*>:
case VariantType::String:
variant_helper<const char*>::set(&mStore, other.get<const char*>());
return *this;
case variant_trait_v<int*>:
case VariantType::ArrayInt:
variant_helper<int*>::set(&mStore, other.get<int*>(), mSize);
return *this;
case variant_trait_v<float*>:
case VariantType::ArrayFloat:
variant_helper<float*>::set(&mStore, other.get<float*>(), mSize);
return *this;
case variant_trait_v<double*>:
case VariantType::ArrayDouble:
variant_helper<double*>::set(&mStore, other.get<double*>(), mSize);
return *this;
case variant_trait_v<bool*>:
case VariantType::ArrayBool:
variant_helper<bool*>::set(&mStore, other.get<bool*>(), mSize);
return *this;
case variant_trait_v<std::string*>:
variant_helper<std::string*>::set(&mStore, other.get<std::string*>(), mSize);
case VariantType::ArrayString:
variant_helper<std::vector<std::string>>::set(&mStore, other.get<std::vector<std::string>>());
return *this;
default:
mStore = other.mStore;
Expand All @@ -200,29 +195,29 @@ Variant& Variant::operator=(Variant&& other) noexcept
mSize = other.mSize;
mType = other.mType;
switch (mType) {
case variant_trait_v<const char*>:
case VariantType::String:
variant_helper<const char*>::set(&mStore, other.get<const char*>());
*reinterpret_cast<char**>(&(other.mStore)) = nullptr;
return *this;
case variant_trait_v<int*>:
case VariantType::ArrayInt:
variant_helper<int*>::set(&mStore, other.get<int*>(), mSize);
*reinterpret_cast<int**>(&(other.mStore)) = nullptr;
return *this;
case variant_trait_v<float*>:
case VariantType::ArrayFloat:
variant_helper<float*>::set(&mStore, other.get<float*>(), mSize);
*reinterpret_cast<float**>(&(other.mStore)) = nullptr;
return *this;
case variant_trait_v<double*>:
case VariantType::ArrayDouble:
variant_helper<double*>::set(&mStore, other.get<double*>(), mSize);
*reinterpret_cast<double**>(&(other.mStore)) = nullptr;
return *this;
case variant_trait_v<bool*>:
case VariantType::ArrayBool:
variant_helper<bool*>::set(&mStore, other.get<bool*>(), mSize);
*reinterpret_cast<bool**>(&(other.mStore)) = nullptr;
return *this;
case variant_trait_v<std::string*>:
variant_helper<std::string*>::set(&mStore, other.get<std::string*>(), mSize);
*reinterpret_cast<std::string**>(&(other.mStore)) = nullptr;
case VariantType::ArrayString:
variant_helper<std::vector<std::string>>::set(&mStore, other.get<std::vector<std::string>>());
*reinterpret_cast<std::vector<std::string>**>(&(other.mStore)) = nullptr;
return *this;
default:
mStore = other.mStore;
Expand Down
12 changes: 6 additions & 6 deletions Framework/Core/src/VariantPropertyTreeHelpers.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ template boost::property_tree::ptree o2::framework::basicVectorToBranch(float*,
template boost::property_tree::ptree o2::framework::basicVectorToBranch(int*, size_t);
template boost::property_tree::ptree o2::framework::basicVectorToBranch(double*, size_t);
template boost::property_tree::ptree o2::framework::basicVectorToBranch(bool*, size_t);
template boost::property_tree::ptree o2::framework::basicVectorToBranch(std::basic_string<char>*, size_t);
template boost::property_tree::ptree o2::framework::basicVectorToBranch(std::basic_string<char> const*, size_t);

template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<float>&& values);
template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<int>&& values);
template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<double>&& values);
template boost::property_tree::ptree o2::framework::vectorToBranch(std::vector<std::string>&& values);
template boost::property_tree::ptree o2::framework::vectorToBranch(float*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(int*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(double*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(bool*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(std::basic_string<char>*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(float const*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(int const*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(double const*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(bool const*, size_t);
template boost::property_tree::ptree o2::framework::vectorToBranch(std::basic_string<char> const*, size_t);

template boost::property_tree::ptree o2::framework::labeledArrayToBranch(o2::framework::LabeledArray<float>&& array);
template boost::property_tree::ptree o2::framework::labeledArrayToBranch(o2::framework::LabeledArray<int>&& array);
Expand Down