此 C++ 代码包含哪些未定义的行为

Question

我是在阅读 Effective C++（第三版）第 11 条后写下这段代码的。

#include <iostream>
using namespace std;

#define MAX_COLORS 20
class Widget
{
 public:
    Widget ( int seed );
    ~Widget ( );
    Widget& operator=( const Widget& rhs );
    void ToString ( );
 private:

    Widget& SelfAssignmentUnsafe ( const Widget& rhs );
    Widget& SelfAssignmentSafe ( const Widget& rhs );
    Widget& SelfAssignmentAndExceptionSafe ( const Widget& rhs );
    void MakeDeepCopy ( const Widget& rhs );
    int *colorPallete;
};

void Widget::ToString()
{
 int i = 0;
 for ( i = 0; i < MAX_COLORS; i++ )
 {
 cout << "colorPallete[" << i << "]: " << colorPallete[i] << endl;
 }
}

Widget::Widget ( int seed ):
    colorPallete ( new int[MAX_COLORS])
    {
     int i = 0;
     for ( i = 0; i < MAX_COLORS; i++ )
     {
      colorPallete[i] = seed + i;
     }
    }

Widget& Widget::operator=( const Widget& rhs )
{
//    return SelfAssignmentUnsafe ( rhs );

//    return SelfAssignmentSafe( rhs ); 

    return SelfAssignmentAndExceptionSafe ( rhs );
}

Widget& Widget::SelfAssignmentUnsafe ( const Widget& rhs )
{
    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy( rhs );
    return *this;
}

Widget& Widget::SelfAssignmentSafe ( const Widget& rhs )
{
    if ( this == &rhs ) return *this;

    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy ( rhs );
    return *this;
}

void Widget::MakeDeepCopy ( const Widget& rhs )
{
    int i = 0;
    colorPallete = new int [MAX_COLORS];
    for ( i = 0;i < MAX_COLORS; i++ )
    {
     colorPallete[i] = rhs.colorPallete[i];
    }
}

Widget& Widget::SelfAssignmentAndExceptionSafe ( const Widget& rhs )
{
    int *origColorPallete = colorPallete;
    MakeDeepCopy ( rhs );
    delete[] origColorPallete;
    origColorPallete = 0;
    return *this;    
}

Widget::~Widget()
{
 delete[] colorPallete;
}    


int main()
{
 Widget b(10);
 Widget a(20);
 b.ToString();
 b = b; 
 cout << endl << "After: " << endl;
 b.ToString();
}

作者谈到在赋值运算符中处理对self的赋值：

Widget a(10);
a = a;

我从 Widget 的赋值运算符调用 Widget::SelfAssignmentAndExceptionSafe。

在 Widget::SelfAssignmentAndExceptionSafe 中的想法是将 colorPallete 指针保存在 origColorPallete 中。然后制作 rhs.colorPallete 的深层副本。当复制成功时，我删除原始指针并返回对自身的引用。

上述机制应该是自赋值和异常安全的。

但是，Widget::SelfAssignmentAndExceptionSafe 无法正确处理对 self 的分配。 colorPallete 数组包含自赋值后的垃圾。它对其他情况的处理非常好。

为什么会这样？

请帮忙。

[编辑：检查所有答案后]

感谢您的回答。我已经更新了 MakeDeepCopy 方法，该示例现在可以正常工作了。下面，我粘贴了更新后的代码：

#include <iostream>

using namespace std;

#define MAX_COLORS 20
class Widget
{
 public:
    Widget ( int seed );
    ~Widget ( );
    Widget& operator=( const Widget& rhs );
    void ToString ( );
 private:
    Widget( Widget& rhs );
    Widget& SelfAssignmentUnsafe ( const Widget& rhs );
    Widget& SelfAssignmentSafe ( const Widget& rhs );
    Widget& SelfAssignmentAndExceptionSafe ( const Widget& rhs );
    void MakeDeepCopy ( const int* rhs );
    int *colorPallete;
};

void Widget::ToString()
{
 int i = 0;
 for ( i = 0; i < MAX_COLORS; i++ )
 {
 cout << "colorPallete[" << i << "]: " << colorPallete[i] << endl;
 }
}

Widget::Widget ( int seed ):
    colorPallete ( new int[MAX_COLORS])
    {
     int i = 0;
     for ( i = 0; i < MAX_COLORS; i++ )
     {
      colorPallete[i] = seed + i;
     }
    }

Widget& Widget::operator=( const Widget& rhs )
{
//    return SelfAssignmentUnsafe ( rhs );

//    return SelfAssignmentSafe( rhs ); 

    return SelfAssignmentAndExceptionSafe ( rhs );
}

Widget& Widget::SelfAssignmentUnsafe ( const Widget& rhs )
{
    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy( rhs.colorPallete );
    return *this;
}

Widget& Widget::SelfAssignmentSafe ( const Widget& rhs )
{
    if ( this == &rhs ) return *this;

    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy ( rhs.colorPallete );
    return *this;
}

void Widget::MakeDeepCopy ( const int* rhs )
{
    int i = 0;
    colorPallete = new int [MAX_COLORS];
    for ( i = 0;i < MAX_COLORS; i++ )
    {
     colorPallete[i] = rhs[i];
    }
}

Widget& Widget::SelfAssignmentAndExceptionSafe ( const Widget& rhs )
{
    int *origColorPallete = colorPallete;
    MakeDeepCopy ( rhs.colorPallete );
    delete[] origColorPallete;
    origColorPallete = 0;
    return *this;    
}

Widget::~Widget()
{
 delete[] colorPallete;
}    


int main()
{
 Widget b(10);
 Widget a(20);
 b.ToString();
 b = b; 
 cout << endl << "After: " << endl;
 b.ToString();
}

[编辑：根据查尔斯的回应修改代码]

这个想法是实现“复制和交换”习语，以使代码既自分配又安全。请注意，复制仅在复制构造函数中实现。如果复制成功，我们交换赋值运算符。

对上一次更新的另一个改进是 MakeDeepCopy 的界面依赖于正确的使用。在调用 MakeDeepCopy 之前，我们必须存储/删除 colorPallete 指针。现在不存在这样的依赖关系。

#include <iostream>

using namespace std;

#define MAX_COLORS 20
class Widget
{
 public:
    Widget ( int seed );
    ~Widget ( );
    Widget& operator=( const Widget& rhs );
    void ToString ( );
    Widget( const Widget& rhs );
 private:
    int *colorPallete;
};

void Widget::ToString()
{
 int i = 0;
 for ( i = 0; i < MAX_COLORS; i++ )
 {
 cout << "colorPallete[" << i << "]: " << colorPallete[i] << endl;
 }
}

Widget::Widget ( int seed ):
    colorPallete ( new int[MAX_COLORS])
    {
     int i = 0;
     for ( i = 0; i < MAX_COLORS; i++ )
     {
      colorPallete[i] = seed + i;
     }
    }

Widget::Widget( const Widget& rhs ):
    colorPallete( new int[MAX_COLORS] )
{
    std::copy ( rhs.colorPallete, rhs.colorPallete + MAX_COLORS, colorPallete );
}

Widget& Widget::operator=( const Widget& rhs )
{
    Widget tmp(rhs);

    std::swap ( colorPallete, tmp.colorPallete );   

    return *this; 
}

Widget::~Widget()
{
 delete[] colorPallete;
}    


int main()
{
 Widget b(10);
 Widget a(20);
 b.ToString();
 b = b; 
 cout << endl << "After: " << endl;
 b.ToString();
}

Answer 1

您看到的垃圾是因为MakeDeepCopy 函数始终从rhs 的colorPallete 成员复制，而不是您在origColorPallete 中制作的复制。

以下修改将解决它：

int *Widget::MakeDeepCopy ( const int *rhs )
{
    int i = 0;
    int *colorPallete = new int [MAX_COLORS];
    for ( i = 0;i < MAX_COLORS; i++ )
    {
     colorPallete[i] = rhs[i];
    }
    return colorPallete;
}

Widget& Widget::SelfAssignmentAndExceptionSafe ( const Widget& rhs )
{
    int *origColorPallete = colorPallete;
    colorPallete = MakeDeepCopy ( origColorPallete );
    delete[] origColorPallete;
    origColorPallete = 0;
    return *this;        
}

实际上，通过上述修改，您可能希望将MakeDeepCopy 重命名为CopyColorPalette 或其他名称（特别是如果您想保留原来的MakeDeepCopy 用于其他目的）。

Answer 2

您可以通过简单地使用 std::vector 而不是动态分配的数组来避免很多这样的麻烦。向量支持赋值（包括自赋值），所以没有什么可做的。

Answer 3

问题是您没有处理复制本身。 因此，当您对自身执行复制时，该语句

colorPallete = new int [MAX_COLORS];

其实也是覆盖了rhs的colorPallete

Answer 4

当你调用 MakeDeepCopy 时，你总是传入对对象的引用。因此它再次作为自分配运行。

如果您检查每个公共方法中的自赋值，并且仅在传递另一个对象的情况下调用赋值时才运行复制，您会好得多。

Answer 5

在您的示例中突出显示的是缺少用户定义的复制构造函数。当您提供用户定义的析构函数和赋值运算符时，可以合理地推断您可能需要用户定义的复制构造函数，这就是这里的实际情况。对编译器生成的复制构造函数的任何显式或隐式调用都将导致原始的最后一个和副本被销毁时的未定义行为。

您可以为您的类编写一个简单的no-throw 交换函数，并且编写一个异常中性 复制构造函数相当容易。 （实际上，我认为编写起来很简单，并且相当容易推断出它是异常中立的。）如果您根据这两个函数（复制和交换习惯用法）来实现赋值运算符，您应该会发现它要容易得多.尤其是，您应该发现不再需要对自我分配进行任何检查。

编辑：

自您的更新以来，您已使 Widget 赋值运算符异常安全。但是，您的设计取决于这样一个事实，即您在分配操作中只有一个操作可能会抛出（新内存的分配），因为ints 的分配不能抛出。一般来说，如果你持有一个对象数组，这不会持有。

我知道MakeDeepCopy 是一个私有函数，但即便如此，它也有一个接口，很大程度上取决于正确使用。成员变量 colorPallete 必须为 delete[]ed 并设置为 0，或者在调用成功时必须将其保存为临时变量，以便随后可以为 delete[]ed。

即使您不想公开复制构造函数，我仍然会使用它来实现赋值运算符，因为它使整个代码更简单。

例如

Widget::Widget( const Widget& rhs )
    : colorPallete( new int[MAX_COLORS] )
{
    // OK because assigning ints won't through
    std::copy( rhs.colorPallete, rhs.colorPallete + MAX_COLORS. colorPallete );
}

Widget& Widget::operator=( const Widget& rhs )
{
    // Try allocating a copy, Widget's copy constructor must
    // leak anything if it throws

    Widget tmp( rhs );

    // If that worked, swap with the copy - this can't throw

    std::swap( colorPallete, tmp.colorPallete );

    // Our old internals are now part of tmp so will be
    // deallocated by tmp's destructor
}

我在复制构造函数中有有效的MakeDeepCopy，但调用代码没有任何必要条件，因为它是一个复制构造函数和一个简单的两行赋值运算符（恕我直言）更明显是异常安全的.

请注意，如果您持有一个在赋值期间可能抛出的对象数组，则您必须做一些更聪明的事情来保持异常安全性和透明度。例如（这可能说明了为什么使用std::vector 是一个好主意）：

template< class T  >
class PartialArrayDeleter
{
public:
    PartialArrayDeleter( T* p )
        : p_( p ) {}

    ~PartialArrayDeleter()
    {
        delete[] p_;
    }

    void reset()
    {
        p_ = 0;
    }

private:
    T* p_;
};

Widget::Widget( const Widget& rhs )
    : colorPallete( new Obj[MAX_COLORS] )
{
    PartialArrayDeleter<Obj> del( colorPallete );

    std::copy( rhs.colorPallete, rhs.colorPallete + MAX_COLORS. colorPallete );

    del.reset();
}

编辑 2：

如果您认为考虑分配 int 以外的对象无关紧要，请注意，如果您只考虑您拥有的类，则在分配期间重新分配并不是绝对必要的。所有小部件在其构造函数中分配的内存量相同。一个简单、高效且异常安全的赋值运算符是：

Widget& Widget::operator=( const Widget& rhs )
{
    for( size_t i = 0; i != MAX_COLORS; ++i )
    {
        colorPallete[i] = rhs.colorPallete[i];
    }
    return *this;
}

ints 的自分配是安全的，并且如前所述，ints 的分配也是异常安全的。 （我不能 100% 确定，但我认为 std::copy 在技术上不能保证自分配副本是安全的。）