I have a problem with the Clustering class, the union within it is causing a problem.
I have tried several things and still nothing, checked the operators, but wasn't able to find the error.
This was built originally with Visual Studio 2008, and it compiles just fine there.
But I must build it with Visual Studio 2019 and encountered this problem during compilation:
error C2280: 'Clustering<T>::NewType::~NewType(void)': attempting to reference a deleted function
error C2280: with
error C2280: [
error C2280: T=ConsortiaRelationElem
error C2280: ]
I imagine this error is due to the implementation of the c 11 parameters.
Here's the definition for the Clustering class:
template <typename T>
class Clustering
{
friend CPlayer;
public:
union NewType
{
struct
{
T _Ttype;
};
CPlayer* m_player;
};
NewType m_Data;
public:
Clustering(){};
Clustering(const T& _Ttype)
{
m_Data.m_player = NULL;
memcpy( &m_Data._Ttype, &_Ttype, sizeof(T) );
};
Clustering& operator=(const T& _Ttype)
{
m_Data.m_player = NULL;
memcpy(&m_Data._Ttype, &_Ttype, sizeof(T));
};
~Clustering() { };
};
This are all the errors i currently get while compiling:
warning C4624: 'Clustering<T>::NewType': destructor was implicitly defined as deleted
warning C4624: with
warning C4624: [
warning C4624: T=ConsortiaRelationElem
warning C4624: ]
error C2280: 'Clustering<T>::NewType::~NewType(void)': attempting to reference a deleted function
error C2280: with
error C2280: [
error C2280: T=ConsortiaRelationElem
error C2280: ]
message : 'Clustering<T>::NewType::~NewType(void)': function was implicitly deleted because 'Clustering<T>::NewType' has a variant data member 'Clustering<T>::NewType::_Ttype' with a non-trivial destructor
- Complete Code: https://shrib.com/#Woodrat5GwaRRM
- Full Build Output: https://shrib.com/#Yaretzi3mo9xQ4
CodePudding user response：
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;
}
};
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.
1.1 using std::variant
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();
};
};
(notice that we can avoid cleaning up m_player, since pointers are trivial.)