移动构造函数导致调试断言失败
我在练习移动构造函数。
我定义了这个类,编写了拷贝构造函数和移动构造函数,但编译器始终使用拷贝构造函数,而不是移动构造函数。
最终,通过插入noexcept关键字,编译器使用了移动构造函数,但它给我一个“Debug Assertion Failed”的错误:
我到底做错了什么?
这是这个类:
//Definition of Class VectorContainer
class VectorContainer
{
private:
double* elem;
int sz;
public:
VectorContainer(int s) :elem{ new double[s] }, sz{ s } { SignifyCreation(); }
~VectorContainer() { delete[] elem; }
void pushBack(double d, int a)
{
elem[a] = d;
cout << "Element " << a << " has been set to " << elem[a] << endl;
}
void SignifyCreation()
{
cout << "A VectorContainer Object has been created " << endl;
}
VectorContainer(const VectorContainer& vc);
VectorContainer* operator=(const VectorContainer& vc);
VectorContainer( VectorContainer&& vc) noexcept; //MOVE Constructor Declared
//double operator[](int s) { return elem[s]; } //Accepts an integer and sets sz to the value of s
const double operator[](int s) const { return elem[s]; } // Subscript Constant
};
//COPY CONSTRUCTOR
VectorContainer::VectorContainer(const VectorContainer& a) : elem{ new double[a.sz] }, sz{ a.sz }
{
for (int i = 0; i != a.sz; i++)
{
cout << "New Element[" << i << "]'s content was = " << elem[i] << endl;
elem[i] = a[i];
cout << "Incoming Parameter " << i << "'s content is = " << a[0] << endl;
cout << "New Element[" << i << "]'s content has now been set to = " << elem[i] << endl;
}
}
//COPY ASSIGNMENT IMPLEMENTATION
//VectorContainer* operator=(const VectorContainer& vc)
VectorContainer* VectorContainer::operator=(const VectorContainer& a)
{
double* p = new double[a.sz];
for (int i = 0; i != a.sz; i++)
{
p[i] = a[i];
}
delete elem;
elem = p;
sz = a.sz;
return this;
}
VectorContainer::VectorContainer(VectorContainer&& vc) noexcept :elem{ vc.elem }, sz{ vc.sz }
{
vc.elem = nullptr;
vc.sz = 0;
cout << "A VectorContainer Object has been MOVED " << endl;
}
VectorContainer& SpitVector()
{
VectorContainer vc10(2);
vc10.pushBack(55.67, 0); vc10.pushBack(78.9, 1);
return vc10;
}
int main()
{
VectorContainer vc1(2);
VectorContainer vc3(2); //To be copy assigned
VectorContainer vc4(2);
//I have created VC1 and initialized its members
//I have created vc2 but not initialized its members. Lemme test.
vc1.pushBack(45.67, 0); vc1.pushBack(20.78, 1);
cout << "vc1 element 1 = " << vc1[0] << endl;
cout << "vc1 element 2 = " << vc1[1] << endl;
VectorContainer vc2(vc1);
cout << "vc2 element 1 = " << vc2[0] << endl;
cout << "vc2 element 2 = " << vc2[1] << endl;
cout << "vc1 element 1 is still " << vc1[0] << endl;
cout << "vc1 element 2 is still " << vc1[1] << endl;
cout << "vc3's element 1 is " << vc3[0] << endl;
cout << "vc3's element 2 is " << vc3[1] << endl;
vc3 = vc2; ///???? Is this the right implementation of the Copy Assigment?
cout << "vc3's element 1 is now " << vc3[0] << endl;
cout << "vc3's element 2 is now " << vc3[1] << endl;
VectorContainer vc5(move(SpitVector()));
}
解决方案
在 VectorContainer::operator= 中,你使用了 delete elem;,这是未定义行为(Undefined Behavior),因为 elem 是用 new[] 分配的,所以你必须改用 delete[] elem;。始终让 new[] 配对 delete[],以及 new 配对 delete。
(一个更好的做法是完全不手动使用 new/delete,但稍后我会进一步讲解。)
此外,VectorContainer::operator= 返回一个 VectorContainer* 指针,但它本应返回一个 VectorContainer& 引用。
请尝试这样:
VectorContainer& operator=(const VectorContainer& vc);
...
VectorContainer& VectorContainer::operator=(const VectorContainer& a)
{
double* p = new double[a.sz];
for (int i = 0; i != a.sz; i++)
{
p[i] = a[i];
}
delete[] elem;
elem = p;
sz = a.sz;
return *this;
}
你还缺少一个移动赋值运算符。既然你正在实现移动构造函数,你也应该实现移动赋值:
VectorContainer& operator=(VectorContainer&& a);
...
VectorContainer& VectorContainer::operator=(VectorContainer&& a)
{
double *p = elem;
elem = a.elem; a.elem = nullptr;
sz = a.sz; a.sz = 0;
delete[] p;
return *this;
}
也就是说,为了更安全的实现,考虑使用拷贝-交换(Copy-and-Swap)惯用法,它利用你现有的拷贝构造函数、移动构造函数和析构函数,这样你就不必在运算符中重复它们的代码:
VectorContainer& VectorContainer::operator=(const VectorContainer& a)
{
// make a temp copy of the source array
VectorContainer temp(a);
// swap the old array with the copied array
std::swap(elem, temp.elem);
std::swap(sz, temp.sz);
// frees the old array when temp goes out of scope
return *this;
}
VectorContainer& VectorContainer::operator=(VectorContainer&& a)
{
// make a temp move of the source array
VectorContainer temp(std::move(a));
// swap the old array with the moved array
std::swap(elem, temp.elem);
std::swap(sz, temp.sz);
// frees the old array when temp goes out of scope
return *this;
}
在这种情况下,你可以把两个运算符合并成一个运算符:
VectorContainer& operator=(VectorContainer a);
...
VectorContainer& VectorContainer::operator=(VectorContainer a)
{
// swap the old array with the copied/moved array
std::swap(elem, a.elem);
std::swap(sz, a.sz);
// frees the old array when the parameter goes out of scope
return *this;
}
通过按值传递参数,调用方在构造该参数时就决定调用拷贝还是移动。该运算符无论哪种情况都只是对结果数组进行所有权的转移。
也就是说,你的代码还存在一些与断言错误或内存管理无关的问题:
SpitVector()返回的是对局部变量的引用。这是未定义行为,因为调用方会接收到一个已释放内存的悬空引用。你可以直接按值返回局部变量,并在调用处去掉std::move()。让编译器为你优化返回值。
cpp
VectorContainer SpitVector()
{
VectorContainer vc10(2);
...
return vc10;
}
cpp
int main()
{
...
VectorContainer vc5(SpitVector());
// or:
// VectorContainer vc5 = SpitVector();
}
* 在 VectorContainer 的拷贝构造函数中,循环中的 a[0] 应该改为 a[i]。你正在从源数组中记录错误的元素。
* 非const版本的 VectorContainer::operator[] 应返回一个 double& 的引用,而不是一个 double 的值。那将允许你通过你的运算符为数组元素赋值:
cpp
double& operator[](int s) { return elem[s]; }
* const版本的 VectorContainer::operator[] 返回一个 double 的值,因此它已经是一个拷贝,在上面再指定 const 是冗余的:
cpp
double operator[](int s) const { return elem[s]; }
* 你的 VectorContainer::pushBack() 方法应该改名。在标准容器中,push_back() 方法是在容器末尾添加一个新元素,但你并不是在这样做。你只是给一个现有元素赋值。在解决上述问题后,你其实可以直接去掉 pushBack(),改用你的 operator[]:
如果你真的想实现正确的推送逻辑,你需要在每次推入新值时重新分配你的数组:
```cpp class VectorContainer { private: double* elem;
int sz;
public: ...
void setValue(double d, int a)
{
elem[a] = d;
cout << "Element " << a << " has been set to " << d << endl;
}
void pushBack(double d)
{
double* p = new double[sz + 1];
for (int i = 0; i != sz; ++i)
{
p[i] = elem[i];
}
p[sz] = d;
delete[] elem;
elem = p;
++sz;
cout << "Element " << d << " has been added" << endl;
}
...
}; ```
最后,一旦你对自己类的手动数组管理感到得心应手,你应该把这一切抛弃,改用 std::vector,它已经实现了具有正确拷贝语义和移动语义的动态数组。编译器就能据此为你生成构造函数、析构函数和赋值运算符的默认实现,并且它们在 std::vector 上的表现将会正确:
#include <vector>
class VectorContainer
{
private:
std::vector<double> elem;
public:
VectorContainer(size_t s) : elem{ s } { SignifyCreation(); }
void setValue(double d, int a)
{
elem[a] = d;
cout << "Element " << a << " has been set to " << d << endl;
}
void pushBack(double d)
{
elem.push_back(d);
cout << "Element " << d << " has been added" << endl;
}
void SignifyCreation()
{
cout << "A VectorContainer Object has been created " << endl;
}
double& operator[](size_t s) { return elem[s]; }
double operator[](size_t s) const { return elem[s]; }
};
