1. non-trivial union members
One of the problems in your code is the union.
unions only implicitly define their special member functions if their members have trivial implimentations for them.
-> Once you add a member to a union that implements a special member function you'll have to manually implement that one for your union.
From the c++ standard:
At most one non-static data member of a union may have a brace-or-equal-initializer. [Note: if any non-static data member of a union has a non-trivial default constructor (12.1), copy constructor (12.8), move constructor (12.8), copy assignment operator (12.8), move assignment operator (12.8), or destructor (12.4), the corresponding member function of the union must be user-provided or it will be implicitly deleted (8.4.3) for the union. — end note ]
Example:
// only trivial members: the compiler will provide all functions for you
union A {
int i;
float f;
double d;
// member functions provided by your compiler
A() = default;
A(const A&) = default;
A(A&&) = default;
A& operator=(const A&) = default;
A& operator=(A&&) = default;
~A() = default;
};
// std::string & std::vector have non-trivial constructors, assignment operators and destructors
// so the member functions will be deleted.
union B {
int i;
std::string s;
std::vector<int> v;
// member functions provided by your compiler
B() = delete;
B(const B&) = delete;
B(B&&) = delete;
B& operator=(const B&) = delete;
B& operator=(B&&) = delete;
~B() = delete;
};
godbolt example with your code
The reasoning behind this is that there is no way the compiler could actually generate those functions. For example the constructor of union B would need to know if it needs to call the constructor of std::string, std::vector or none at all - but that depends on which member you initialize which the compiler doesn't know yet.
Same thing for the destructor of union B - which destructor should it call? std::string::~string() ? or std::vector<int>::~vector()? or none at all in the int case? The union doesn't know which member is the active one so there's no way to provide a sensible destructor.
So the compiler leaves the responsibility of actually implementing all those special functions up to you.
A very simple example of implementing a two-type generic union could look like this:
template<class T, class U>
struct variant {
private:
enum {
IS_NONE,
IS_T,
IS_U
} activeMember;
union {
T t;
U u;
char dummy;
};
public:
variant() : activeMember(IS_NONE), dummy() {}
variant(T const& _t): activeMember(IS_T), t(_t) {}
variant(U const& _u): activeMember(IS_U), u(_u) {}
variant(variant const& other): variant() { *this = other; }
variant& operator=(variant const& other) {
if(&other != this) {
reset();
activeMember = other.activeMember;
if(other.activeMember == IS_T)
new (&t) T(other.t);
else if(other.activeMember == IS_U)
new (&u) U(other.u);
}
return *this;
}
variant& operator=(T const& _t) {
reset();
activeMember = IS_T;
new (&t) T(_t);
return *this;
}
variant& operator=(U const& _u) {
reset();
activeMember = IS_U;
new (&u) U(_u);
return *this;
}
explicit operator T const&() {
if(activeMember != IS_T) throw std::domain_error("variant is not T!");
return t;
}
explicit operator U const&() {
if(activeMember != IS_U) throw std::domain_error("variant is not U!");
return u;
}
explicit operator bool() {
return activeMember != IS_NONE;
}
~variant() { reset(); }
void reset() {
if(activeMember == IS_T)
t.~T();
else if(activeMember == IS_U)
u.~U();
activeMember = IS_NONE;
}
};
godbolt example
Which could be used like this:
variant<int, std::string> v = 12;
v = "MIAU";
Notice that we need to initialize one of the union members in the constructors, as well as call their respective destructors as needed.
If we want to switch the active member after construction we also need to destruct the previosly active union member first, then use placement-new to construct the new value.
If you can use C++17 i'd recommend using std::variant instead of the union, since that basically does the exact same thing as the variant class i posted above and more.
2. memcpy of non-trival types
You use memcpy( &m_Data._Ttype, &_Ttype, sizeof(T) ); both in the constructor and assignment operator of your class.
You are only allowed to copy objects with memcpy if they are trivially copyable, i.e.:
static_assert(std::is_trivially_copyable<T>::value, "T is not trivially copyable!");
Given the compile error you got your T is most likely not trivially copyable, so using memcpy in this case leads to undefined behaviour.
You can instead use placement new to call the copy- or move constructor of T, e.g.:
template<class T>
Clustering<T>& Clustering<T>::operator=(T const& t)
{
new (&m_Data._Ttype) T(t);
return *this;
};
Also don't forget to call the destructor for your T (if it's not trivially destructable) in your ~Clustering() destructor (otherwise you leak your T!):
template<class T>
Clustering<T>::~Clustering() {
if(/* union actually contains T */)
m_Data._Ttype.~T();
}
- Full working example
In your example you could write Clustering like this:
template <typename T>
class Clustering
{
friend CPlayer;
public:
union
{
T _Ttype;
CPlayer* m_player;
};
bool containsT;
public:
Clustering() : containsT(false), m_player(nullptr) {};
Clustering(T const& t) : containsT(true), _Ttype(t) {};
Clustering& operator=(T const& t)
{
// cleanup old T if needed
if(containsT)
_Ttype.~T();
containsT = true;
new (&_Ttype) T(t);
return *this;
};
~Clustering() {
if(containsT)
_Ttype.~T();
};
};
working godbolt example
(notice that we can avoid cleaning up m_player, since pointers are trivial.)
