Copy operation modifying source operand

Copy operation modifies data member of source object

Description

This defect occurs when a copy constructor or copy assignment operator modifies a mutable data member of its source operand.

For instance, this copy constructor A modifies the data member m of its source operand other:

class A {
  mutable int m;
   
public:
 ...
  A(const A &other) : m(other.m) {
    other.m = 0; //Modification of source
  }
}

Risk

A copy operation with a copy constructor (or copy assignment operator):

className new_object = old_object; //Calls copy constructor of className

copies its source operand old_object to its destination operand new_object. After the operation, you expect the destination operand to be a copy of the unmodified source operand. If the source operand is modified during copy, this assumption is violated.

Fix

Do not modify the source operand in the copy operation.

If you are modifying the source operand in a copy constructor to implement a move operation, use a move constructor instead. Move constructors are defined in the C++11 standard and later.

Examples

Copy Constructor Modifying Source

#include <algorithm>
#include <vector>
 
class A {
  mutable int m;
   
public:
  A() : m(0) {}
  explicit A(int m) : m(m) {}
   
  A(const A &other) : m(other.m) {
    other.m = 0;
  }
   
  A& operator=(const A &other) {
    if (&other != this) {
      m = other.m;
      other.m = 0;
    }
    return *this;
  }
   
  int get_m() const { return m; }
};
 
void f() {
  std::vector<A> v{10};
  A obj(12);
  std::fill(v.begin(), v.end(), obj);
}

In this example, a vector of ten objects of type A is created. The std::fill function copies an object of type A, which has a data member with value 12, to each of the ten objects. After this operation, you might expect that all ten objects in the vector have a data member with value 12.

However, the first copy modifies the data member of the source to the value 0. The remaining nine copies copy this value. After the std::fill call, the first object in the vector has a data member with value 12 and the remaining objects have data members with value 0.

Correction — Use Move Constructor for Modifying Source

Do not modify data members of the source operand in a copy constructor or copy assignment operator. If you want your class to have a move operation, use a move constructor instead of a copy constructor.

In this corrected example, the copy constructor and copy assignment operator of class A do not modify the data member m. A separate move constructor modifies the source operand.

#include <algorithm>
#include <vector>
 
class A {
  int m;
   
public:
  A() : m(0) {}
  explicit A(int m) : m(m) {}
   
  A(const A &other) : m(other.m) {}
  A(A &&other) : m(other.m) { other.m = 0; }
   
  A& operator=(const A &other) {
    if (&other != this) {
      m = other.m;
    }
    return *this;
  }
  
  //Move constructor
  A& operator=(A &&other) {
    m = other.m;
    other.m = 0;
    return *this;
  }
   
  int get_m() const { return m; }
};
 
void f() {
  std::vector<A> v{10};
  A obj(12);
  std::fill(v.begin(), v.end(), obj);
}

Result Information

Group: Object Oriented

Language: C++

Default: On for handwritten code, off for generated code

Command-Line Syntax: COPY_MODIFYING_SOURCE

Impact: Medium