2

Consider the following scenario

class Integer
{
   long long n;
   public:
   Integer(long long i):n(i){}
   Integer(){cout<<"constructor";}
   void print()
   {
      cout<<n<<endl;
   }
   Integer(const Integer &a){cout<<"copy constructor"<<" "<<a.n<<endl;}
   Integer operator+(Integer b);
};

Integer Integer:: operator+(Integer b)
{
   this->n = this->n + b.n;
   return *this;
}
int main() 
{
   // your code goes here
   Integer a(5);
   Integer b(6);
   Integer c(a+b);//line 1
   return 0;
 }

If a+b was temporary then i understand that copy constructor would not be called. But a+b does not return a temporary. The output i get is

 copy constructor 6  //this is because Integer object is passed as value to operator+
 copy constructor -5232903157125162015 //this is when object is returned by value from operator+

I think there should be one more call when a+b is used to initialize c. Most of the related questions have to do with return value optimization but i can't relate RVO to this.

Caduchon
  • 4,574
  • 4
  • 26
  • 67
Prashant Bhanawat
  • 101
  • 1
  • 1
  • 7
  • Unrelated; Shouldn't you write `b.n += this->n; return b;`? – Mohit Jain May 20 '16 at 06:02
  • 3
    Off topic: `a+b` is kinda broken. It modifies `a` the way a += operator would. – user4581301 May 20 '16 at 06:07
  • `a+b` *does* return a temporary (since `operator+` returns by value) and the call is removed by *copy elision*. – M.M May 20 '16 at 06:08
  • http://stackoverflow.com/questions/12953127/what-are-copy-elision-and-return-value-optimization – M.M May 20 '16 at 06:09
  • You have undefined behaviour. You can't really reason a program with UB. At least, you can't use it as an example to illustrate something. – juanchopanza May 20 '16 at 06:10
  • 1
    BTW the garbage in your output is because your copy constructor does not actually initialize `n`, it just displays the value it is supposed to copy – M.M May 20 '16 at 06:10

4 Answers4

2

You say a+b does not return a temporary. Wrong. a+b (even if it changes a which will certainly mislead an future reader...) returns a temporary copy of a because it is declared to return an Integer and not a reference (Integer&).

So here is what happens:

Integer a(5); // creates an integer from int 5
Integer b(6); // creates an integer from int 6

Integer c(a+b); /* first creates a temp using copy ctor from b
                   updates a
                   creates a temp copy of the result
                   should create an integer by copy of the result (elided)
                   destroys the temporary created from b
*/

return 0; // destroys c, b, a

BTW: you had forgotten to initialize n in your copy ctor - it should be:

Integer(const Integer &a):n(a.n) {cout<<"copy constructor"<<" "<<a.n<<endl;}
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
1

You are witnessing the copy ellission of the constructor.

When a nameless temporary, not bound to any references, would be moved or copied into an object of the same type (ignoring top-level cv-qualification), the copy/move is omitted. When that temporary is constructed, it is constructed directly in the storage where it would otherwise be moved or copied to. When the nameless temporary is the argument of a return statement, this variant of copy elision is known as RVO, "return value optimization".

To view the expected output (not eliding of constructor) use -fno-elide-constructors option in gcc.

EDIT

As mentioned in my comment on your question Integer:: operator+ is not quite right. Moreover as mentioned in other answers, your copy constructor doesn't do the initialization of member and thus causes undefined behaviuor.

Mohit Jain
  • 30,259
  • 8
  • 73
  • 100
1

To answer your question, the copy constructor never copies anything and the + operator isn't overloaded correctly. Additionally, the default constructor leaves the Integer class with an uninitialized member, which may cause undefined behaviors (not 100% on that one) which may be causing some issues.

So what happens is, because the copy constructors never actually copy a value to the n member of *this, the default constructor is called, and std::cout displays whatever is at the location of n of 'a.n'; which, if uninitialized, could be anything.

This website from Caltech is good, if imperfect, reference for overloading operators.

Try this.

#include <iostream>

using namespace std;


class Integer{
    long long n;
public:
    Integer(long long i):n(i){}
    //Integer(){cout<<"cosntructor";} //This creates an unitialized  instance of integer.

    Integer(){// Do this instead.
        n = 0;
        cout << "Constructor" << endl;
    }

    void print(){
        cout<<n<<endl;
    }
    //Integer(const Integer &a){cout<<"copy constructor"<<" "<<a.n<<endl;} // This isn't a copy contructor.

    Integer(const Integer &a){ //This is a copy contructor.
        this->n = a.n;
        cout << "Copy constructor" << " " << a.n << endl;
    }

    Integer& operator+=(const Integer &);
    Integer operator+(const Integer &);
};

Integer& Integer::operator+=(const Integer &b){ //Always overload incrementors/decrementors first, makes life easier.
    this->n = this->n + b.n;
    return *this;
}


Integer Integer:: operator+(const Integer &b){
     return Integer(*this)+=b.n; //Notice the use of overloaded incrementor inside the '+' operator.
}

int main(){
    // your code goes here
    Integer a(5);
    Integer b(6);
    Integer c(a+b);//line 1
    return 0;
}
NonCreature0714
  • 5,744
  • 10
  • 30
  • 52
  • It isn't clear how you're answering the question. This might require some clarification. – juanchopanza May 20 '16 at 06:16
  • `operator+` would be better off if you flipped `this` and `b` because you're modifying `this`: `return b.n += this->n;` – user4581301 May 20 '16 at 06:19
  • @juanchopanza Okay, I added some clarification. – NonCreature0714 May 20 '16 at 06:21
  • @user4581301 That would inhibit NRVO. Better in two statements, finishing with `return b;`. – juanchopanza May 20 '16 at 06:22
  • @NonCreature0714 I still can't see it, but maybe it is because the question is unclear. – juanchopanza May 20 '16 at 06:23
  • @user4581301 Ah yes, but I don't want to return `*this`, I want to return a new Integer object with `*this` passed to the copy constructor, then incremented, using this website as a reference: http://users.cms.caltech.edu/~donnie/cs11/cpp/cpp-ops.html – NonCreature0714 May 20 '16 at 06:23
  • @NonCreature0714 That reference seems to offer bad advice on a few things. I would ignore it. – juanchopanza May 20 '16 at 06:25
  • @juanchopanza Ok, I'll take your word for it because your rep is WAY up there – but what specifically is bad? I'm always looking for a good learning opportunity. – NonCreature0714 May 20 '16 at 06:27
  • @juanchopanza - I don't want to get too chatty, but yeah the original post could be cleaned up a little – NonCreature0714 May 20 '16 at 06:29
  • `return Integer(*this)+=b.n;` inhibits return value optimization, resulting in needless copying. Also your `+=` should return a reference. – juanchopanza May 20 '16 at 06:29
  • @juanchopanza point taken. Non, just spotted the extra copy construction. You're safe, but you have an extra construction. `b` was copied on the way in, so you might as well use it and not waste its construction. – user4581301 May 20 '16 at 06:30
  • @user4581301 Ah wait, if `b` is copied on the way in, `return b;` cannot be elided anyway (but if the argument is a temporary, the copy *into* `b` can.) – juanchopanza May 20 '16 at 06:31
  • @juanchopanza Thanks for the guidance, I'll update in a second – NonCreature0714 May 20 '16 at 06:33
  • @user4581301 I think I just spotted the extra copy construction too, I'll update it where I think it's happening – NonCreature0714 May 20 '16 at 06:37
0

It has a correct number of copy ctor calls:

1st is for b because operator+(Integer b) accepts Integer by value.

2nd is for c created by operator+ result.

By the standard there could be a 3rd call: return *this might have created a temporary which would later be copied to c. But in reality it is always elided. You can turn off this elision in gcc&clang and you can't turn it off in MSVC.

ixSci
  • 13,100
  • 5
  • 45
  • 79