Prepare Attributable for virtual inheritance

Use only zero-param constructors to avoid diamond initialization
pitfalls

include/openPMD/Iteration.hpp

@@ -244,19 +244,25 @@ class Iteration : public Attributable
 private:
     Iteration();
 
-    std::shared_ptr<internal::IterationData> m_iterationData{
-        new internal::IterationData};
+    using Data_t = internal::IterationData;
+    std::shared_ptr<Data_t> m_iterationData;
 
-    inline internal::IterationData const &get() const
+    inline Data_t const &get() const
     {
         return *m_iterationData;
     }
 
-    inline internal::IterationData &get()
+    inline Data_t &get()
     {
         return *m_iterationData;
     }
 
+    inline void setData(std::shared_ptr<Data_t> data)
+    {
+        m_iterationData = std::move(data);
+        Attributable::setData(m_iterationData);
+    }
+
     void flushFileBased(
         std::string const &, IterationIndex_t, internal::FlushParams const &);
     void flushGroupBased(IterationIndex_t, internal::FlushParams const &);

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,23 +429,21 @@ 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;
+    std::shared_ptr<Data_t> m_recordComponentData;
+
+    RecordComponent();
 
-    inline internal::RecordComponentData const &get() const
+    inline Data_t const &get() const
     {
         return *m_recordComponentData;
     }
 
-    inline internal::RecordComponentData &get()
+    inline Data_t &get()
     {
         return *m_recordComponentData;
     }

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();
 
@@ -543,9 +544,10 @@ OPENPMD_private
     using iterations_t = decltype(internal::SeriesData::iterations);
     using iterations_iterator = iterations_t::iterator;
 
-    std::shared_ptr<internal::SeriesData> m_series = nullptr;
+    using Data_t = internal::SeriesData;
+    std::shared_ptr<Data_t> m_series = nullptr;
 
-    inline internal::SeriesData &get()
+    inline Data_t &get()
     {
         if (m_series)
         {
@@ -558,7 +560,7 @@ OPENPMD_private
         }
     }
 
-    inline internal::SeriesData const &get() const
+    inline Data_t const &get() const
     {
         if (m_series)
         {
@@ -571,6 +573,13 @@ OPENPMD_private
         }
     }
 
+    inline void setData(std::shared_ptr<internal::SeriesData> series)
+    {
+        m_series = std::move(series);
+        iterations = m_series->iterations;
+        Attributable::setData(m_series);
+    }
+
     std::unique_ptr<ParsedInput> parseInput(std::string);
     /**
      * @brief Parse non-backend-specific configuration in JSON config.

include/openPMD/backend/Attributable.hpp

@@ -111,14 +111,16 @@ class Attributable
     friend class WriteIterations;
 
 protected:
-    std::shared_ptr<internal::AttributableData> m_attri{
-        new internal::AttributableData()};
+    // tag for internal constructor
+    struct NoInit
+    {};
 
-    // Should not be called publicly, only by implementing classes
-    Attributable(std::shared_ptr<internal::AttributableData>);
+    using Data_t = internal::AttributableData;
+    std::shared_ptr<Data_t> m_attri;
 
 public:
     Attributable();
+    Attributable(NoInit);
 
     virtual ~Attributable() = default;
 

include/openPMD/backend/BaseRecord.hpp

@@ -63,25 +63,23 @@ 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;
 
-    inline internal::BaseRecordData<T_elem> &get()
+    inline Data_t const &get() const
     {
         return *m_baseRecordData;
     }
 
-    inline internal::BaseRecordData<T_elem> const &get() const
+    inline Data_t &get()
     {
         return *m_baseRecordData;
     }
 
     BaseRecord();
 
 protected:
-    BaseRecord(std::shared_ptr<internal::BaseRecordData<T_elem> >);
-
-    inline void setData(internal::BaseRecordData<T_elem> *data)
+    inline void setData(std::shared_ptr<Data_t> data)
     {
         m_baseRecordData = std::move(data);
         Container<T_elem>::setData(m_baseRecordData);
@@ -137,7 +135,6 @@ class BaseRecord : public Container<T_elem>
     bool scalar() const;
 
 protected:
-    BaseRecord(internal::BaseRecordData<T_elem> *);
     void readBase();
 
 private:
@@ -164,25 +161,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>(Attributable::NoInit())
 {
-    Container<T_elem>::setData(m_baseRecordData);
+    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,29 +102,27 @@ class BaseRecordComponent : public Attributable
     ChunkTable availableChunks();
 
 protected:
-    std::shared_ptr<internal::BaseRecordComponentData>
-        m_baseRecordComponentData{new internal::BaseRecordComponentData()};
+    using Data_t = internal::BaseRecordComponentData;
+    std::shared_ptr<Data_t> m_baseRecordComponentData;
 
-    inline internal::BaseRecordComponentData const &get() const
+    inline Data_t const &get() const
     {
         return *m_baseRecordComponentData;
     }
 
-    inline internal::BaseRecordComponentData &get()
+    inline Data_t &get()
     {
         return *m_baseRecordComponentData;
     }
 
-    inline void setData(std::shared_ptr<internal::BaseRecordComponentData> data)
+    inline void setData(std::shared_ptr<Data_t> data)
     {
         m_baseRecordComponentData = std::move(data);
         Attributable::setData(m_baseRecordComponentData);
     }
 
-    BaseRecordComponent(std::shared_ptr<internal::BaseRecordComponentData>);
-
-private:
     BaseRecordComponent();
+    BaseRecordComponent(NoInit);
 }; // BaseRecordComponent
 
 namespace detail

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,7 +147,7 @@ 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()};
+    std::shared_ptr<ContainerData> m_containerData;
 
     inline void setData(std::shared_ptr<ContainerData> containerData)
     {
@@ -428,10 +428,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,14 +450,13 @@ OPENPMD_protected
         flushAttributes(flushParams);
     }
 
-    // clang-format off
-OPENPMD_private
-    // clang-format on
-
-    Container() : Attributable{nullptr}
+    Container() : Attributable(NoInit())
     {
-        Attributable::setData(m_containerData);
+        setData(std::make_shared<ContainerData>());
     }
+
+    Container(NoInit) : Attributable(NoInit())
+    {}
 };
 
 namespace internal
@@ -532,7 +527,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)

include/openPMD/backend/PatchRecordComponent.hpp

@@ -112,29 +112,27 @@ OPENPMD_private
      */
     bool dirtyRecursive() const;
 
-    std::shared_ptr<internal::PatchRecordComponentData>
-        m_patchRecordComponentData{new internal::PatchRecordComponentData()};
-
-    PatchRecordComponent();
-
     // clang-format off
 OPENPMD_protected
     // clang-format on
 
-    PatchRecordComponent(std::shared_ptr<internal::PatchRecordComponentData>);
+    using Data_t = internal::PatchRecordComponentData;
+
+    std::shared_ptr<Data_t> m_patchRecordComponentData;
+
+    PatchRecordComponent();
 
-    inline internal::PatchRecordComponentData const &get() const
+    inline Data_t const &get() const
     {
         return *m_patchRecordComponentData;
     }
 
-    inline internal::PatchRecordComponentData &get()
+    inline Data_t &get()
     {
         return *m_patchRecordComponentData;
     }
 
-    inline void
-    setData(std::shared_ptr<internal::PatchRecordComponentData> data)
+    inline void setData(std::shared_ptr<Data_t> data)
     {
         m_patchRecordComponentData = std::move(data);
         BaseRecordComponent::setData(m_patchRecordComponentData);

src/Iteration.cpp

@@ -36,9 +36,9 @@ namespace openPMD
 using internal::CloseStatus;
 using internal::DeferredParseAccess;
 
-Iteration::Iteration() : Attributable{nullptr}
+Iteration::Iteration() : Attributable(NoInit())
 {
-    Attributable::setData(m_iterationData);
+    setData(std::make_shared<Data_t>());
     setTime(static_cast<double>(0));
     setDt(static_cast<double>(1));
     setTimeUnitSI(1);