0

I am Trying to make an overload in "+" operator. The problem is I have to create a brand new object that will contain the sum, but when I try to return it, it gets deleted.(I think its because the object is locally created, so the system makes a shallow copy, and when it gets out of scope its content is deleted) How can I avoid it getting deleted?

The operator overload

Thing Thing::operator+(const Thing& thing){
Thing sum;
sum.attribute=this->attribute+thing.attribute;
sum.things= new int[sum.attribute];
static int k=0;
for(int i=0;i<this->attribute;i++){
    std::cout<<this->things[i];
    sum.things[k]=this->things[i];
}
for(int j=0;j<thing.attribute;j++){
    std::cout<<thing.things[j];
    sum.things[k]=thing.things[j];
}
return sum;} 

The main

int main(){
    Thing one(1,1);
    Thing two(2,2);
    Thing three;
    three = one+two;
    std::cout<<"One"<<std::endl;
    std::cout<<one.getAttribute()<<std::endl;
    one.showVec();
    std::cout<<"Two"<<std::endl;
    std::cout<<two.getAttribute()<<std::endl;
    two.showVec();
    std::cout<<"Three"<<std::endl;
    std::cout<<three.getAttribute()<<std::endl;
    three.showVec();

Copy constructor

Thing::Thing(const Thing& stuff){
    std::cout<<"Copy"<<std::endl;
    this->attribute=stuff.attribute;
    this->things = new int[this->attribute];
    for(int i =0;i<attribute;i++)
        this->things[i]=stuff.things[i];
    
}

The class

class Thing {
    private:
        int attribute;
        int* things;
    public:
        //constructors
        Thing(int=0,int=0);
        Thing(const Thing&);
        //destructor
        ~Thing();
        //operators
        Thing operator+(const Thing&);
        //getters
        int getAttribute();
        //functions
        void showVec();

};

My output

Default

Default

Default

Default

122Destructor

One

1

1

Two

2

2 2

Three

3

17002888 16974016 0 (The problem is here)

Destructor

Destructor

Destructor

  • In the first section of code, you are not incrementing `k`. (This is why your `three` array contains garbage after the first element.) Also, why is it `static`? You have not included the implementations of your constructor and destructor, so it's hard to say what else may be wrong. – Bungo Feb 19 '21 at 07:07
  • A function always returns an [rvalue](https://stackoverflow.com/questions/9406121/what-exactly-is-an-r-value-in-c), which gets [move-assigned](https://en.cppreference.com/w/cpp/language/move_assignment) to `three`, and then it is deleted. This is perfectly normal and nothing to worry about. To avoid your pointer being deleted, you overload the move assignment operator to prepare for this. – IWonderWhatThisAPIDoes Feb 19 '21 at 07:11
  • You are using the compiler-generated copy assignment operator, which is almost certainly not what you want. This operator will copy the pointer `things`, not the array to which it points. So, after `three = one + two`, the pointer `three.things` is a copy of the pointer `temp.things`, where by `temp` I mean the unnamed object returned by `one + two`. When `temp` is destroyed, its destructor presumably destroys the array `temp.things` was pointing to. Since `three.things` was pointing to that same array, it now points to garbage. – Bungo Feb 19 '21 at 07:16
  • You haven't followed the rule of three as you have not defined a copy assignment operator. – john Feb 19 '21 at 07:26
  • 1
    Alternatively, use `std::vector` and follow the rule of zero – Caleth Feb 19 '21 at 09:02

1 Answers1

0

An object returned by a function will always be an rvalue. This object will be deleted when it goes out of use, which is usualy immediately. Your expression three = one + two takes that returned rvalue, and move-assigns it to three. After that, the rvalue is deleted.

The problem with your code is that the rvalue deletes its pointer when its destructor is called, and because you have not overloaded the move assignment, it is shallow-copied to three. That results in three holding an invalid pointer.

To avoid this problem, simply overload the move assignment (and eventually other operators that are needed, like the move constructor) to reflect this.

Thing& Thing::operator=(Thing&& rvalue) {
    if(things) { delete things; things = nullptr } // delete pointer if any (it would otherwise get overwritten and leak)
    attribute = rvalue.attribute;
    things = rvalue.things;
    rvalue.things = nullptr; // zero rvalue's pointer so that the object cannot delete it
}

Thing::~Thing() {
    if(things) { delete things; } // first make sure the pointer is there
}
IWonderWhatThisAPIDoes
  • 1,000
  • 1
  • 4
  • 14