Skip to content

Commit 969efba

Browse files
committed
Fix default constructors/operators of Container and BaseRecordComponent
These derive virtually from Attributable, and this avoids that pitfalls propagate to user code.
1 parent 4b2c420 commit 969efba

File tree

2 files changed

+90
-0
lines changed

2 files changed

+90
-0
lines changed

include/openPMD/backend/BaseRecordComponent.hpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,51 @@ class BaseRecordComponent : virtual public Attributable
6767
friend class Container;
6868

6969
public:
70+
/*
71+
* Need to define these manually due to the virtual inheritance from
72+
* Attributable.
73+
* Otherwise, they would only run from the most derived class
74+
* if explicitly called.
75+
* If not defining these, a user could destroy copy/move constructors/
76+
* assignment operators by deriving from any class that has a virtual
77+
* Attributable somewhere.
78+
* Care must be taken in move constructors/assignment operators to not move
79+
* multiple times (which could happen in diamond inheritance situations).
80+
*/
81+
82+
BaseRecordComponent(BaseRecordComponent const &other)
83+
: Attributable(NoInit())
84+
{
85+
m_attri = other.m_attri;
86+
m_baseRecordComponentData = other.m_baseRecordComponentData;
87+
}
88+
89+
BaseRecordComponent(BaseRecordComponent &&other) : Attributable(NoInit())
90+
{
91+
if (other.m_attri)
92+
{
93+
m_attri = std::move(other.m_attri);
94+
}
95+
m_baseRecordComponentData = std::move(other.m_baseRecordComponentData);
96+
}
97+
98+
BaseRecordComponent &operator=(BaseRecordComponent const &other)
99+
{
100+
m_attri = other.m_attri;
101+
m_baseRecordComponentData = other.m_baseRecordComponentData;
102+
return *this;
103+
}
104+
105+
BaseRecordComponent &operator=(BaseRecordComponent &&other)
106+
{
107+
if (other.m_attri)
108+
{
109+
m_attri = std::move(other.m_attri);
110+
}
111+
m_baseRecordComponentData = std::move(other.m_baseRecordComponentData);
112+
return *this;
113+
}
114+
70115
double unitSI() const;
71116

72117
BaseRecordComponent &resetDatatype(Datatype);

include/openPMD/backend/Container.hpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,51 @@ OPENPMD_protected
457457

458458
Container(NoInit) : Attributable(NoInit())
459459
{}
460+
461+
public:
462+
/*
463+
* Need to define these manually due to the virtual inheritance from
464+
* Attributable.
465+
* Otherwise, they would only run from the most derived class
466+
* if explicitly called.
467+
* If not defining these, a user could destroy copy/move constructors/
468+
* assignment operators by deriving from any class that has a virtual
469+
* Attributable somewhere.
470+
* Care must be taken in move constructors/assignment operators to not move
471+
* multiple times (which could happen in diamond inheritance situations).
472+
*/
473+
474+
Container(Container const &other) : Attributable(NoInit())
475+
{
476+
m_attri = other.m_attri;
477+
m_containerData = other.m_containerData;
478+
}
479+
480+
Container(Container &&other) : Attributable(NoInit())
481+
{
482+
if (other.m_attri)
483+
{
484+
m_attri = std::move(other.m_attri);
485+
}
486+
m_containerData = std::move(other.m_containerData);
487+
}
488+
489+
Container &operator=(Container const &other)
490+
{
491+
m_attri = other.m_attri;
492+
m_containerData = other.m_containerData;
493+
return *this;
494+
}
495+
496+
Container &operator=(Container &&other)
497+
{
498+
if (other.m_attri)
499+
{
500+
m_attri = std::move(other.m_attri);
501+
}
502+
m_containerData = std::move(other.m_containerData);
503+
return *this;
504+
}
460505
};
461506

462507
namespace internal

0 commit comments

Comments
 (0)