From 55baecd7d00dfd6ecf1293f98ca1132343161a69 Mon Sep 17 00:00:00 2001 From: Greg Sjaardema Date: Mon, 18 Sep 2023 13:49:37 -0600 Subject: [PATCH 1/3] IOSS: Make Ioss_Property.h use variant instead of union --- .../seacas/libraries/ioss/src/Ioss_Property.C | 136 +++++++----------- .../seacas/libraries/ioss/src/Ioss_Property.h | 30 ++-- .../libraries/ioss/src/Ioss_PropertyManager.C | 2 +- 3 files changed, 68 insertions(+), 100 deletions(-) diff --git a/packages/seacas/libraries/ioss/src/Ioss_Property.C b/packages/seacas/libraries/ioss/src/Ioss_Property.C index e5635885c8..41584d329e 100644 --- a/packages/seacas/libraries/ioss/src/Ioss_Property.C +++ b/packages/seacas/libraries/ioss/src/Ioss_Property.C @@ -45,9 +45,8 @@ namespace { * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, int value, Origin origin) - : name_(std::move(name)), type_(INTEGER), origin_(origin) + : name_(std::move(name)), type_(INTEGER), origin_(origin), data_(value) { - data_.ival = value; } /** \brief Create an INTEGER type property using an int64_t variable. @@ -57,9 +56,8 @@ Ioss::Property::Property(std::string name, int value, Origin origin) * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, int64_t value, Origin origin) - : name_(std::move(name)), type_(INTEGER), origin_(origin) + : name_(std::move(name)), type_(INTEGER), origin_(origin), data_(value) { - data_.ival = value; } /** \brief Create a REAL type property. @@ -69,9 +67,8 @@ Ioss::Property::Property(std::string name, int64_t value, Origin origin) * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, double value, Origin origin) - : name_(std::move(name)), type_(REAL), origin_(origin) + : name_(std::move(name)), type_(REAL), origin_(origin), data_(value) { - data_.rval = value; } /** \brief Create a STRING type property. @@ -81,9 +78,8 @@ Ioss::Property::Property(std::string name, double value, Origin origin) * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, const std::string &value, Origin origin) - : name_(std::move(name)), type_(STRING), origin_(origin) + : name_(std::move(name)), type_(STRING), origin_(origin), data_(value) { - data_.sval = new std::string(value); } /** \brief Create a VEC_INTEGER type property. @@ -93,9 +89,8 @@ Ioss::Property::Property(std::string name, const std::string &value, Origin orig * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, const std::vector &value, Origin origin) - : name_(std::move(name)), type_(VEC_INTEGER), origin_(origin) + : name_(std::move(name)), type_(VEC_INTEGER), origin_(origin), data_(value) { - data_.ivec = new std::vector(value); } /** \brief Create a VEC_DOUBLE type property. @@ -105,9 +100,8 @@ Ioss::Property::Property(std::string name, const std::vector &value, Origin * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, const std::vector &value, Origin origin) - : name_(std::move(name)), type_(VEC_DOUBLE), origin_(origin) + : name_(std::move(name)), type_(VEC_DOUBLE), origin_(origin), data_(value) { - data_.dvec = new std::vector(value); } /** \brief Create a STRING type property from const char* argument. @@ -117,9 +111,8 @@ Ioss::Property::Property(std::string name, const std::vector &value, Ori * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, const char *value, Origin origin) - : name_(std::move(name)), type_(STRING), origin_(origin) + : name_(std::move(name)), type_(STRING), origin_(origin), data_(std::string(value)) { - data_.sval = new std::string(value); } /** \brief Create a POINTER type property. @@ -129,9 +122,8 @@ Ioss::Property::Property(std::string name, const char *value, Origin origin) * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, void *value, Origin origin) - : name_(std::move(name)), type_(POINTER), origin_(origin) + : name_(std::move(name)), type_(POINTER), origin_(origin), data_(value) { - data_.pval = value; } /** \brief Set implicit property with a specified type. @@ -143,50 +135,7 @@ Ioss::Property::Property(std::string name, void *value, Origin origin) Ioss::Property::Property(const Ioss::GroupingEntity *ge, std::string name, const BasicType type) : name_(std::move(name)), type_(type), origin_(IMPLICIT) { - data_.ge = ge; -} - -/** \brief Copy constructor. - * - * \param[in] from The Ioss::Property to copy - */ -Ioss::Property::Property(const Ioss::Property &from) - : name_(from.name_), type_(from.type_), origin_(from.origin_) -{ - if (!is_implicit() && type_ == STRING) { - data_.sval = new std::string(*(from.data_.sval)); - } - else if (!is_implicit() && type_ == VEC_DOUBLE) { - data_.dvec = new std::vector(*(from.data_.dvec)); - } - else if (!is_implicit() && type_ == VEC_INTEGER) { - data_.ivec = new std::vector(*(from.data_.ivec)); - } - else { - data_ = from.data_; - } -} - -Ioss::Property::~Property() -{ - if (!is_implicit() && type_ == STRING) { - delete data_.sval; - } - else if (!is_implicit() && type_ == VEC_DOUBLE) { - delete data_.dvec; - } - else if (!is_implicit() && type_ == VEC_INTEGER) { - delete data_.ivec; - } -} - -Ioss::Property &Ioss::Property::operator=(Ioss::Property rhs) -{ - std::swap(this->name_, rhs.name_); - std::swap(this->type_, rhs.type_); - std::swap(this->origin_, rhs.origin_); - std::swap(this->data_, rhs.data_); - return *this; + data_ = ge; } bool Ioss::Property::operator==(const Ioss::Property &rhs) const @@ -199,6 +148,10 @@ bool Ioss::Property::operator==(const Ioss::Property &rhs) const return false; } + if (this->origin_ != rhs.origin_) { + return false; + } + switch (this->type_) { case INVALID: break; case REAL: @@ -248,7 +201,6 @@ bool Ioss::Property::operator==(const Ioss::Property &rhs) const } break; } - return true; } @@ -342,12 +294,14 @@ bool Ioss::Property::get_value(int64_t *value) const { bool valid_request = type_ == INTEGER; if (is_explicit()) { - *value = data_.ival; + assert(std::holds_alternative(data_)); + *value = std::get(data_); } else { - const Ioss::GroupingEntity *ge = data_.ge; - const Ioss::Property implicit = ge->get_implicit_property(name_); - valid_request = implicit.get_value(value); + assert(std::holds_alternative(data_)); + const auto *ge = std::get(data_); + const auto implicit = ge->get_implicit_property(name_); + valid_request = implicit.get_value(value); } return valid_request; } @@ -356,12 +310,14 @@ bool Ioss::Property::get_value(double *value) const { bool valid_request = type_ == REAL; if (is_explicit()) { - *value = data_.rval; + assert(std::holds_alternative(data_)); + *value = std::get(data_); } else { - const Ioss::GroupingEntity *ge = data_.ge; - const Ioss::Property implicit = ge->get_implicit_property(name_); - valid_request = implicit.get_value(value); + assert(std::holds_alternative(data_)); + const auto *ge = std::get(data_); + const auto implicit = ge->get_implicit_property(name_); + valid_request = implicit.get_value(value); } return valid_request; } @@ -370,12 +326,14 @@ bool Ioss::Property::get_value(std::string *value) const { bool valid_request = type_ == STRING; if (is_explicit()) { - *value = *(data_.sval); + assert(std::holds_alternative(data_)); + *value = std::get(data_); } else { - const Ioss::GroupingEntity *ge = data_.ge; - const Ioss::Property implicit = ge->get_implicit_property(name_); - valid_request = implicit.get_value(value); + assert(std::holds_alternative(data_)); + const auto *ge = std::get(data_); + const auto implicit = ge->get_implicit_property(name_); + valid_request = implicit.get_value(value); } return valid_request; } @@ -384,12 +342,15 @@ bool Ioss::Property::get_value(std::vector *value) const { bool valid_request = type_ == VEC_INTEGER; if (is_explicit()) { - std::copy(data_.ivec->begin(), data_.ivec->end(), std::back_inserter(*value)); + assert(std::holds_alternative>(data_)); + auto ivec = std::get>(data_); + std::copy(ivec.begin(), ivec.end(), std::back_inserter(*value)); } else { - const Ioss::GroupingEntity *ge = data_.ge; - const Ioss::Property implicit = ge->get_implicit_property(name_); - valid_request = implicit.get_value(value); + assert(std::holds_alternative(data_)); + const auto *ge = std::get(data_); + const auto implicit = ge->get_implicit_property(name_); + valid_request = implicit.get_value(value); } return valid_request; } @@ -398,12 +359,15 @@ bool Ioss::Property::get_value(std::vector *value) const { bool valid_request = type_ == VEC_DOUBLE; if (is_explicit()) { - std::copy(data_.dvec->begin(), data_.dvec->end(), std::back_inserter(*value)); + assert(std::holds_alternative>(data_)); + auto dvec = std::get>(data_); + std::copy(dvec.begin(), dvec.end(), std::back_inserter(*value)); } else { - const Ioss::GroupingEntity *ge = data_.ge; - const Ioss::Property implicit = ge->get_implicit_property(name_); - valid_request = implicit.get_value(value); + assert(std::holds_alternative(data_)); + const auto *ge = std::get(data_); + const auto implicit = ge->get_implicit_property(name_); + valid_request = implicit.get_value(value); } return valid_request; } @@ -412,12 +376,14 @@ bool Ioss::Property::get_value(void *&value) const { bool valid_request = type_ == POINTER; if (is_explicit()) { - value = data_.pval; + assert(std::holds_alternative(data_)); + value = std::get(data_); } else { - const Ioss::GroupingEntity *ge = data_.ge; - const Ioss::Property implicit = ge->get_implicit_property(name_); - valid_request = implicit.get_value(value); + assert(std::holds_alternative(data_)); + const auto *ge = std::get(data_); + const auto implicit = ge->get_implicit_property(name_); + valid_request = implicit.get_value(value); } return valid_request; } diff --git a/packages/seacas/libraries/ioss/src/Ioss_Property.h b/packages/seacas/libraries/ioss/src/Ioss_Property.h index 9e3e26f2a2..93c96df3db 100644 --- a/packages/seacas/libraries/ioss/src/Ioss_Property.h +++ b/packages/seacas/libraries/ioss/src/Ioss_Property.h @@ -1,4 +1,4 @@ -// Copyright(C) 1999-2022 National Technology & Engineering Solutions +// Copyright(C) 1999-2023 National Technology & Engineering Solutions // of Sandia, LLC (NTESS). Under the terms of Contract DE-NA0003525 with // NTESS, the U.S. Government retains certain rights in this software. // @@ -46,10 +46,10 @@ namespace Ioss { // To set implicit property Property(const GroupingEntity *ge, std::string name, BasicType type); - Property(const Property &from); - Property &operator=(Property rhs); + Property(const Property &from) = default; + Property &operator=(Property &rhs); - ~Property(); + ~Property() = default; std::string get_string() const; int64_t get_int() const; @@ -101,6 +101,15 @@ namespace Ioss { bool operator!=(const Ioss::Property &rhs) const; bool operator==(const Ioss::Property &rhs) const; + friend void swap(Ioss::Property &first, Ioss::Property &second) // nothrow + { + using std::swap; + swap(first.name_, second.name_); + swap(first.type_, second.type_); + swap(first.origin_, second.origin_); + swap(first.data_, second.data_); + } + private: std::string name_{}; BasicType type_{INVALID}; @@ -118,15 +127,8 @@ namespace Ioss { /// The actual value of the property. Use 'type_' to /// discriminate the actual type of the property. - union Data { - std::string *sval; - void *pval{nullptr}; - const GroupingEntity *ge; - double rval; - int64_t ival; - std::vector *dvec; - std::vector *ivec; - }; - Data data_{}; + std::variant, + std::vector, void *> + data_; }; } // namespace Ioss diff --git a/packages/seacas/libraries/ioss/src/Ioss_PropertyManager.C b/packages/seacas/libraries/ioss/src/Ioss_PropertyManager.C index 0392e5616d..a9797beb73 100644 --- a/packages/seacas/libraries/ioss/src/Ioss_PropertyManager.C +++ b/packages/seacas/libraries/ioss/src/Ioss_PropertyManager.C @@ -34,7 +34,7 @@ void Ioss::PropertyManager::add(const Ioss::Property &new_prop) if (iter != m_properties.end()) { m_properties.erase(iter); } - m_properties.insert(ValuePair(new_prop.get_name(), new_prop)); + m_properties.emplace(new_prop.get_name(), new_prop); } /** \brief Checks if a property exists in the database. From 4589bd5c0af486c3c681e98a989cb3cc848ed072 Mon Sep 17 00:00:00 2001 From: Greg Sjaardema Date: Mon, 18 Sep 2023 13:56:56 -0600 Subject: [PATCH 2/3] IOSS: Add missing include --- packages/seacas/libraries/ioss/src/Ioss_Property.h | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/seacas/libraries/ioss/src/Ioss_Property.h b/packages/seacas/libraries/ioss/src/Ioss_Property.h index 93c96df3db..2a76a0a99b 100644 --- a/packages/seacas/libraries/ioss/src/Ioss_Property.h +++ b/packages/seacas/libraries/ioss/src/Ioss_Property.h @@ -10,6 +10,7 @@ #include // for int64_t #include // for string +#include #include namespace Ioss { From 788f39ed2a646d017aaa42b607aab4eebfba193a Mon Sep 17 00:00:00 2001 From: Greg Sjaardema Date: Mon, 18 Sep 2023 14:48:17 -0600 Subject: [PATCH 3/3] IOSS: Fix ambiguous argument error on intel --- packages/seacas/libraries/ioss/src/Ioss_Property.C | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/seacas/libraries/ioss/src/Ioss_Property.C b/packages/seacas/libraries/ioss/src/Ioss_Property.C index 41584d329e..f5858906f2 100644 --- a/packages/seacas/libraries/ioss/src/Ioss_Property.C +++ b/packages/seacas/libraries/ioss/src/Ioss_Property.C @@ -45,7 +45,7 @@ namespace { * \param[in] origin The origin of the property - IMPLICIT, or EXTERNAL, or ATTRIBUTE */ Ioss::Property::Property(std::string name, int value, Origin origin) - : name_(std::move(name)), type_(INTEGER), origin_(origin), data_(value) + : name_(std::move(name)), type_(INTEGER), origin_(origin), data_(static_cast(value)) { }