C++ return by value - what happens with the pointer inside?

Question

So I'm having a bit of SegFaults and was wondering if someone can explain me in higher depth how does this work.

I have a wrapper class around a numeric variable, and I'm trying to build a compute tree of the expression. Here is a sample code:

class DoubleWrap{ 
public:
   static int id;
   DoubleWrap(double value){this->value = value;myid=++id;}
   double value;
   int myid;
   std::vector<DoubleWrap*> parents;
   DoubleWrap operator+(DoubleWrap& x){
      DoubleWrap result(this->value + x.value);
      setParents(*this,result);
      setParents(x,result);
      return result;
   }
   void setParents(DoubleWrap& parent,DoubleWrap& child){
      child.parents.push_back(&parent);
   }
   void printTree(){
      std::cout<< "[" << this->myid << "]-(";
      for (auto& elem: this->parents)
          std::cout<< elem->myid << "," << elem <<",";
      std::cout<<")"<<std::endl;
      for (auto& elem: this->parents)
          elem->printTree();
  }
}

The problem I got is for an expression like x = a+b+c+d; When I try to look for the parents of x, it has 2 parents, but parents[0].parents[0] is nil, e.g. for some reason the pointer to the allocated place does not work or the allocated memory is destroyed?

I'm quite interested if anyone can advise me on a workaround and why this happens.

To give an example on this code:

DoubleWrap dw(12.6);
DoubleWrap dw2(12.5);
DoubleWrap dw3(12.4);
DoubleWrap dw4(12.3);
DoubleWrap a = dw + dw2 + dw3;
DoubleWrap b = a + dw+ dw2+ dw3 + dw4;
b.printTree();

I expect the output:

[10]-(9,0x13a4b50,4,0x7fff5bdb62f0,)
[9]-(8,0x13a4b00,3,0x7fff5bdb62c0,)
[8]-(7,0x13a49f0,2,0x7fff5bdb6290,)
[7]-(6,0x7fff5bdb6320,1,0x7fff5bdb6260,)
[6]-(5,0x13a4db0,3,0x7fff5bdb62c0,)
[5]-(1,0x7fff5bdb6260,2,0x7fff5bdb6290,)
[1]-()
[2]-()
[3]-()
[1]-()
[2]-()
[3]-()
[4]-()

But the result I get (usually different on different runs):

[10]-(9,0x7ffffd5e2f00,4,0x7ffffd5e2de0,)
[9]-(33,0x1c6da10,3,0x7ffffd5e2db0,)
Process finished with exit code 139

My guess is that the return of the operator in fact copies the DoubleWrap variable, thus value to which the pointer in parents is pointing to goes out of scope and the memory is released?

The temporary solution was to return by reference, but the question is why and is there a proper one?

PS: Fixed a previous mistake I got which is a mistake in setParents.

Answer 1

Your problem is that you're adding pointers to temporary objects to the vector.

DoubleWrap a = dw + dw2 + dw3;

The above will first call dw.operator+(dw2). That creates a temporary object, let's name it temp. It then calls temp.operator+(dw3). Now inside operator+ there's this call to setParents where you pass temp as the parent. You then take the address of temp (at least that's what the code should be doing in order to compile) and add it to the vector of child.

Now when the call to operator+ returns, temp gets destroyed, and the vector in a contains a pointer to a non-existing object.

---

You could solve this by storing the elements in the vector using shared_ptr. Note that if referential cycles are possible you should be careful to break those cycles with weak_ptr (I didn't include an implementation for that in the below code). Here's what the final code could look like:

class DoubleWrap {
public:
   static int id;
   DoubleWrap(double value) : value(value), myid(++id) {}
   double value;
   int myid;
   std::vector<std::shared_ptr<DoubleWrap>> parents;
   DoubleWrap operator+(const DoubleWrap& x) const {
      DoubleWrap result(this->value + x.value);
      setParents(*this, result);
      setParents(x, result);
      return result;
   }
   static void setParents(const DoubleWrap& parent, DoubleWrap& child) {
      child.parents.push_back(std::make_shared<DoubleWrap>(parent)); // makes a copy of parent
   }
   void printTree() const {
      std::cout << "[" << this->myid << "]-(";
      for (const auto& elem: this->parents)
         std::cout << elem->myid << "," << elem.get() << ",";
      std::cout << ")" << std::endl;
      for (const auto& elem: this->parents)
         elem->printTree();
   }
};

int DoubleWrap::id = 0;

Answer 2

Your code : DoubleWrap operator+(DoubleWrap& x){ DoubleWrap result(this->value + x->value); would be correct if x was a pointer to a DoubleWrap class but you have coded x as a DoubleWrap class object and not a pointer so your code should read : DoubleWrap operator+(DoubleWrap& x){ DoubleWrap result(this->value + x.value);