Prepare Attributable for virtual inheritance

Only one instance of std::shared<Data_t>, dynamically casted
Use only zero-param constructors to avoid diamond initialization
pitfalls

Author: franzpoeschel

include/openPMD/Iteration.hpp

@@ -244,17 +244,14 @@ class Iteration : public Attributable
 private:
     Iteration();
 
-    std::shared_ptr<internal::IterationData> m_iterationData{
-        new internal::IterationData};
-
     inline internal::IterationData const &get() const
     {
-        return *m_iterationData;
+        return dynamic_cast<internal::IterationData const &>(*m_attri);
     }
 
     inline internal::IterationData &get()
     {
-        return *m_iterationData;
+        return dynamic_cast<internal::IterationData &>(*m_attri);
     }
 
     void flushFileBased(

include/openPMD/RecordComponent.hpp

@@ -105,8 +105,6 @@ class RecordComponent : public BaseRecordComponent
     friend class ParticleSpecies;
     template <typename T_elem>
     friend class BaseRecord;
-    template <typename T_elem>
-    friend class BaseRecordInterface;
     friend class Record;
     friend class Mesh;
     template <typename>
@@ -431,31 +429,23 @@ class RecordComponent : public BaseRecordComponent
      */
     bool dirtyRecursive() const;
 
-    std::shared_ptr<internal::RecordComponentData> m_recordComponentData{
-        new internal::RecordComponentData()};
-
-    RecordComponent();
-
     // clang-format off
 OPENPMD_protected
     // clang-format on
 
-    RecordComponent(std::shared_ptr<internal::RecordComponentData>);
+    using Data_t = internal::RecordComponentData;
 
-    inline internal::RecordComponentData const &get() const
-    {
-        return *m_recordComponentData;
-    }
+    RecordComponent();
 
-    inline internal::RecordComponentData &get()
+    inline Data_t const &get() const
     {
-        return *m_recordComponentData;
+        return dynamic_cast<Data_t const &>(*m_attri);
     }
 
-    inline void setData(std::shared_ptr<internal::RecordComponentData> data)
+    inline Data_t &get()
     {
-        m_recordComponentData = std::move(data);
-        BaseRecordComponent::setData(m_recordComponentData);
+        auto &res = dynamic_cast<Data_t &>(*m_attri);
+        return res;
     }
 
     void readBase();

include/openPMD/Series.hpp

@@ -63,8 +63,13 @@ namespace internal
      *
      * (Not movable or copyable)
      *
+     * Class is final since our std::shared_ptr<Data_t> pattern has the little
+     * disadvantage that child constructors overwrite the parent constructors.
+     * Since the SeriesData constructor does some initialization, making this
+     * class final avoids stumbling over this pitfall.
+     *
      */
-    class SeriesData : public AttributableData
+    class SeriesData final : public AttributableData
     {
     public:
         explicit SeriesData() = default;
@@ -192,10 +197,6 @@ class Series : public Attributable
     friend class internal::SeriesData;
     friend class WriteIterations;
 
-protected:
-    // Should not be called publicly, only by implementing classes
-    Series(std::shared_ptr<internal::SeriesData>);
-
 public:
     explicit Series();
 
@@ -533,13 +534,11 @@ OPENPMD_private
     using iterations_t = decltype(internal::SeriesData::iterations);
     using iterations_iterator = iterations_t::iterator;
 
-    std::shared_ptr<internal::SeriesData> m_series = nullptr;
-
     inline internal::SeriesData &get()
     {
-        if (m_series)
+        if (m_attri)
         {
-            return *m_series;
+            return dynamic_cast<internal::SeriesData &>(*m_attri);
         }
         else
         {
@@ -550,9 +549,9 @@ OPENPMD_private
 
     inline internal::SeriesData const &get() const
     {
-        if (m_series)
+        if (m_attri)
         {
-            return *m_series;
+            return dynamic_cast<internal::SeriesData const &>(*m_attri);
         }
         else
         {
@@ -561,6 +560,12 @@ OPENPMD_private
         }
     }
 
+    inline void setData(std::shared_ptr<internal::SeriesData> series)
+    {
+        iterations = series->iterations;
+        Attributable::setData(std::move(series));
+    }
+
     std::unique_ptr<ParsedInput> parseInput(std::string);
     /**
      * @brief Parse non-backend-specific configuration in JSON config.

include/openPMD/backend/Attributable.hpp

@@ -114,9 +114,6 @@ class Attributable
     std::shared_ptr<internal::AttributableData> m_attri{
         new internal::AttributableData()};
 
-    // Should not be called publicly, only by implementing classes
-    Attributable(std::shared_ptr<internal::AttributableData>);
-
 public:
     Attributable();
 
@@ -356,9 +353,11 @@ OPENPMD_protected
         return m_attri->m_writable;
     }
 
-    inline void setData(std::shared_ptr<internal::AttributableData> attri)
+    template <typename T = internal::AttributableData>
+    inline void setData(std::shared_ptr<T> attri)
     {
-        m_attri = std::move(attri);
+        m_attri = std::static_pointer_cast<internal::AttributableData>(
+            std::move(attri));
     }
 
     inline internal::AttributableData &get()

include/openPMD/backend/BaseRecord.hpp

@@ -63,30 +63,21 @@ class BaseRecord : public Container<T_elem>
     friend class Record;
     friend class Mesh;
 
-    std::shared_ptr<internal::BaseRecordData<T_elem> > m_baseRecordData{
-        new internal::BaseRecordData<T_elem>()};
+    using Data_t = internal::BaseRecordData<T_elem>;
+    std::shared_ptr<Data_t> m_baseRecordData{new Data_t()};
 
-    inline internal::BaseRecordData<T_elem> &get()
+    inline Data_t const &get() const
     {
-        return *m_baseRecordData;
+        return dynamic_cast<Data_t const &>(*this->m_attri);
     }
 
-    inline internal::BaseRecordData<T_elem> const &get() const
+    inline Data_t &get()
     {
-        return *m_baseRecordData;
+        return dynamic_cast<Data_t &>(*this->m_attri);
     }
 
     BaseRecord();
 
-protected:
-    BaseRecord(std::shared_ptr<internal::BaseRecordData<T_elem> >);
-
-    inline void setData(internal::BaseRecordData<T_elem> *data)
-    {
-        m_baseRecordData = std::move(data);
-        Container<T_elem>::setData(m_baseRecordData);
-    }
-
 public:
     using key_type = typename Container<T_elem>::key_type;
     using mapped_type = typename Container<T_elem>::mapped_type;
@@ -137,7 +128,6 @@ class BaseRecord : public Container<T_elem>
     bool scalar() const;
 
 protected:
-    BaseRecord(internal::BaseRecordData<T_elem> *);
     void readBase();
 
 private:
@@ -164,25 +154,20 @@ namespace internal
     template <typename T_elem>
     BaseRecordData<T_elem>::BaseRecordData()
     {
-        Attributable impl{{this, [](auto const *) {}}};
+        Attributable impl;
+        impl.setData({this, [](auto const *) {}});
         impl.setAttribute(
             "unitDimension",
             std::array<double, 7>{{0., 0., 0., 0., 0., 0., 0.}});
     }
 } // namespace internal
 
 template <typename T_elem>
-BaseRecord<T_elem>::BaseRecord() : Container<T_elem>{nullptr}
+BaseRecord<T_elem>::BaseRecord()
 {
-    Container<T_elem>::setData(m_baseRecordData);
+    Attributable::setData(std::make_shared<Data_t>());
 }
 
-template <typename T_elem>
-BaseRecord<T_elem>::BaseRecord(
-    std::shared_ptr<internal::BaseRecordData<T_elem> > data)
-    : Container<T_elem>{data}, m_baseRecordData{std::move(data)}
-{}
-
 template <typename T_elem>
 inline typename BaseRecord<T_elem>::mapped_type &
 BaseRecord<T_elem>::operator[](key_type const &key)

include/openPMD/backend/BaseRecordComponent.hpp

@@ -35,7 +35,7 @@ namespace openPMD
 {
 namespace internal
 {
-    class BaseRecordComponentData : public AttributableData
+    class BaseRecordComponentData : virtual public AttributableData
     {
     public:
         /**
@@ -61,7 +61,7 @@ namespace internal
     };
 } // namespace internal
 
-class BaseRecordComponent : public Attributable
+class BaseRecordComponent : virtual public Attributable
 {
     template <typename T, typename T_key, typename T_container>
     friend class Container;
@@ -102,28 +102,18 @@ class BaseRecordComponent : public Attributable
     ChunkTable availableChunks();
 
 protected:
-    std::shared_ptr<internal::BaseRecordComponentData>
-        m_baseRecordComponentData{new internal::BaseRecordComponentData()};
+    using Data_t = internal::BaseRecordComponentData;
 
-    inline internal::BaseRecordComponentData const &get() const
+    inline Data_t const &get() const
     {
-        return *m_baseRecordComponentData;
+        return dynamic_cast<Data_t const &>(*m_attri);
     }
 
-    inline internal::BaseRecordComponentData &get()
+    inline Data_t &get()
     {
-        return *m_baseRecordComponentData;
+        return dynamic_cast<Data_t &>(*m_attri);
     }
 
-    inline void setData(std::shared_ptr<internal::BaseRecordComponentData> data)
-    {
-        m_baseRecordComponentData = std::move(data);
-        Attributable::setData(m_baseRecordComponentData);
-    }
-
-    BaseRecordComponent(std::shared_ptr<internal::BaseRecordComponentData>);
-
-private:
     BaseRecordComponent();
 }; // BaseRecordComponent
 

include/openPMD/backend/Container.hpp

@@ -65,7 +65,7 @@ namespace internal
         typename T,
         typename T_key = std::string,
         typename T_container = std::map<T_key, T> >
-    class ContainerData : public AttributableData
+    class ContainerData : virtual public AttributableData
     {
     public:
         using InternalContainer = T_container;
@@ -128,7 +128,7 @@ template <
     typename T,
     typename T_key = std::string,
     typename T_container = std::map<T_key, T> >
-class Container : public Attributable
+class Container : virtual public Attributable
 {
     static_assert(
         std::is_base_of<Attributable, T>::value,
@@ -147,22 +147,14 @@ class Container : public Attributable
     using ContainerData = internal::ContainerData<T, T_key, T_container>;
     using InternalContainer = T_container;
 
-    std::shared_ptr<ContainerData> m_containerData{new ContainerData()};
-
-    inline void setData(std::shared_ptr<ContainerData> containerData)
-    {
-        m_containerData = std::move(containerData);
-        Attributable::setData(m_containerData);
-    }
-
     inline InternalContainer const &container() const
     {
-        return m_containerData->m_container;
+        return dynamic_cast<ContainerData const &>(*m_attri).m_container;
     }
 
     inline InternalContainer &container()
     {
-        return m_containerData->m_container;
+        return dynamic_cast<ContainerData &>(*m_attri).m_container;
     }
 
 public:
@@ -428,10 +420,6 @@ class Container : public Attributable
 OPENPMD_protected
     // clang-format on
 
-    Container(std::shared_ptr<ContainerData> containerData)
-        : Attributable{containerData}, m_containerData{std::move(containerData)}
-    {}
-
     void clear_unchecked()
     {
         if (written())
@@ -454,13 +442,9 @@ OPENPMD_protected
         flushAttributes(flushParams);
     }
 
-    // clang-format off
-OPENPMD_private
-    // clang-format on
-
-    Container() : Attributable{nullptr}
+    Container()
     {
-        Attributable::setData(m_containerData);
+        Attributable::setData(std::make_shared<ContainerData>());
     }
 };
 
@@ -532,7 +516,8 @@ namespace internal
         ~EraseStaleEntries()
         {
             auto &map = m_originalContainer.container();
-            using iterator_t = typename BareContainer_t::const_iterator;
+            using iterator_t =
+                typename BareContainer_t::InternalContainer::const_iterator;
             std::vector<iterator_t> deleteMe;
             deleteMe.reserve(map.size() - m_accessedKeys.size());
             for (iterator_t it = map.begin(); it != map.end(); ++it)