c++ Overwriting an already defined variable

Question

I have the following main function, creating a product of coefficients using pointers. It's only a small part of the project, which is used to create polynomials:

#include "header.h"
int main()
{
    TermProd x = TermProd( new Coeff(4), new Coeff(8));
    x.print(); cout << endl;
    x = TermProd( new Coeff(8), new Coeff(15));
    x.print(); cout << endl;
    return 0;
}

After testing, the overwriting seems to be working. But when I call the print on x, I get a segmentation fault. I have been trying and staring at it for quite a while now, but I can't figure out the real problem. Also my searches didn't result into the right direction so I decided to create a small code-snippet which reproduces the error.

My header.h file looks like:

class Term{
public:
    Term(){};
    virtual ~Term(){};
        virtual Term* clone() const = 0;
    virtual void print() const = 0;
};

class Coeff:public Term{
    int m_val; //by def: >=0
public:
    // Constructor
    Coeff();
    Coeff(int val = 1):m_val(val)
    // Copy constructor
    Coeff* clone() const{return new Coeff(this->val());}
    // Destructor
    ~Coeff(){}
    // Accessors
    int val() const{return m_val;} ;
    // Print
    void print() const{cout << m_val; }
};

class TermProd:public Term{
    TermPtr m_lhs, m_rhs;
public:
    // Constructor
    TermProd(TermPtr lhs, TermPtr rhs):m_lhs(lhs), m_rhs(rhs){ }
    // Copy constructor
    TermProd* clone() const
    {
        return new TermProd(m_lhs->clone(), m_rhs->clone());
    }
    // Destructor
    ~TermProd(){ delete m_lhs;delete m_rhs;}
    // Accessors
    Term* lhs() const { return m_lhs; }
    Term* rhs() const { return m_rhs; } 
    // Print
    void print() const
    {
        cout << "("; m_lhs->print(); cout << '*'; m_rhs->print(); cout << ")";
    }       
 };

Answer 1

Note here you are not overwriting the x variable but instead assigning to it. This will invoke the default operator= for your type which roughly causes the following code to be executed

1. The constructor TermProd::TermProd(TermPtr, TermPtr) is executed
2. The values m_lhs and m_rhs are copied into the value x
3. The destructor for the value created in step #1 is now run and m_lhs and m_rhs are deleted

At this point you have a real problem. After step #2 both the value x and the temporary value created in step #1 shared the same m_lhs and m_rhs values. The destructor at step #3 deletes them yet x still has a reference to them which is now effectively pointing at dead memory

In order to fix this problem you will need to add your own operator= which correctly handles the assignment semantics. For example

TermProd& operator=(const TermProd& other) {
  if (&other != this) {
    delete m_lhs;
    delete m_rhs;

    m_lhs = other.m_lhs->clone();
    m_rhs = other.m_rhs->clone();
  }
  return *this;
};

In order to be correct for all scenarios you'll also need to add a proper copy constructor

TermProd::TermProd(const TermProd& other) :
  m_lhs(other.m_lhs->clone()),
  m_rhs(other.m_rhs->clone())
{

}

Really though to make this extremely simple you should consider using std::shared_ptr<Term> as the value for TermPtr. It's a pointer that will make the sharing work without all of the above mentioned overhead