0

I have the following class in C++

class.h

typedef std::complex<float> fcomp;

class wf {
    public:
        int nx, ny, nf; //dimensions
        size_t wfSize;

        fcomp * val; //data

        //constructors
        wf(int nx, int ny, int nf);
        wf(int nx, int ny, int nf, fcomp * data);

        //copy constructor
        wf(const wf & wfOther);

        //copy assignment operators
        wf & operator=(wf & wfOther);
        wf & operator=(const wf & wfOther);

        //move constructor
        wf(wf && wfOther);

        //move assignment operator
        wf & operator=(wf && wfOther);

        //destructor
        ~wf();

        void dispInfo();

        inline wf & operator+=(const wf & wfOther);
};

wf operator+(const wf & a, const wf & b);

and all the definitions in the script file class.c.

#include "class.h"

//----------------------------------------------------------------------------

wf::wf(int nx, int ny, int nf)
    : nx(nx), ny(ny), nf(nf) {

#ifdef debugClasses
    std::cout << "wf: constructor 1" << std::endl;
#endif

    wfSize = nf*nx*ny;

    val = new fcomp[wfSize];
}

//----------------------------------------------------------------------------

wf::wf(int nx, int ny, int nf, fcomp * data)
    : nx(nx), ny(ny), nf(nf){

#ifdef debugClasses
    std::cout << "wf: constructor 2" << std::endl;
#endif

    wfSize = nf*nx*ny;

    val = new fcomp[wfSize];

    for(unsigned int i=0; i<wfSize; i++)
        val[i] = data[i];
}

//----------------------------------------------------------------------------

wf::wf(const wf & wfOther)
    : nx(wfOther.nx), ny(wfOther.ny), nf(wfOther.nf),
    wfSize(wfOther.wfSize) {

#ifdef debugClasses
    std::cout << "wf: copy constructor" << std::endl;
#endif

    val = new fcomp[wfSize];

    memcpy(val, wfOther.val, wfSize*sizeof(fcomp));
}

//----------------------------------------------------------------------------

wf & wf::operator=(const wf & wfOther){

#ifdef debugClasses
    std::cout << "wf: copy assignment operator 1" << std::endl;
#endif

    nx = wfOther.nx;
    ny = wfOther.ny;
    nf = wfOther.nf;
    wfSize = wfOther.wfSize;

    val = new fcomp[wfSize];

    memcpy(val, wfOther.val, wfSize*sizeof(fcomp));

    return *this;
}

//----------------------------------------------------------------------------

wf & wf::operator=(wf & wfOther){

#ifdef debugClasses
    std::cout << "wf: copy assignment operator 2" << std::endl;
#endif

    nx = wfOther.nx;
    ny = wfOther.ny;
    nf = wfOther.nf;
    wfSize = wfOther.wfSize;

    val = new fcomp[wfSize];

    memcpy(val, wfOther.val, wfSize*sizeof(fcomp));

    return *this;
}

//----------------------------------------------------------------------------

wf::wf(wf && wfOther)
    : nx(wfOther.nx), ny(wfOther.ny), nf(wfOther.nf), 
    wfSize(wfOther.wfSize) {

#ifdef debugClasses
    std::cout << "wf: move constructor" << std::endl;
#endif

    val = wfOther.val;

    wfOther.val = nullptr;
    wfOther.nx = 0;
    wfOther.ny = 0;
    wfOther.nf = 0;
    wfOther.wfSize = 0;
}

//----------------------------------------------------------------------------

wf & wf::operator=(wf && wfOther){

#ifdef debugClasses
    std::cout << "wf: move assignment operator" << std::endl;
#endif

    if(this != &wfOther){

        delete [] val;

        val = wfOther.val;
        nx = wfOther.nx;
        ny = wfOther.ny;
        nf = wfOther.nf;
        wfSize = wfOther.wfSize;


        wfOther.val = nullptr;
        wfOther.nx = 0;
        wfOther.ny = 0;
        wfOther.nf = 0;
        wfOther.wfSize = 0;
    }

    return *this;
}

//----------------------------------------------------------------------------

wf::~wf(){

#ifdef debugClasses
    std::cout << "wf: destructor" << std::endl;
#endif

    delete [] val;
}

//----------------------------------------------------------------------------

void wf::dispInfo(){
    std::cout << "nx = " << nx << std::endl;
    std::cout << "ny = " << ny << std::endl;
    std::cout << "nf = " << nf << std::endl;
    std::cout << "wfSize = " << wfSize << std::endl;
}

//----------------------------------------------------------------------------

inline wf & wf::operator+=(const wf & wfOther){
    this->nx += wfOther.nx;
    this->ny += wfOther.ny;
    this->nf += wfOther.nf;
    this->wfSize += wfOther.wfSize;

    return *this;
}

//----------------------------------------------------------------------------

wf operator+(const wf & a, const wf & b){

#ifdef debugClasses
    std::cout << "wf: classes addition" << std::endl;
#endif

    wf c(a.nx,a.ny,a.nf);

    if( a.nx != b.nx || a.ny != b.ny || a.nf != b.nf ){
        std::cout << "wf: invalid classes addition\n";
        throw "invalid-operation";
        exit(1);
    }

    for(unsigned int i=0; i<c.wfSize; i++)
            c.val[i] = a.val[i] + b.val[i];

    return c;
}

Since my class has a pointer variable *val, I define explicitly my constructors, the destructor and also the copy/move constructor and copy/move assignment operators. I have the following piece of code in my main program:

main.c

int main(int argc, char * argv[]){

    wf wfA(8,8,4);
    for(unsigned int i=0; i<wfA.wfSize; i++){
        float fl = (float)(i);
        wfA.val[i] = fcomp(fl,0.0);
    }

    wf wfB(wfA);

    wf wfC = std::move(wfA + wfB);

    wfC.dispInfo();

    for(unsigned int i=0; i<5; i++) 
        std::cout << wfC.val[i] << " ";

return 0;
}

so essentially I create a class object wfA using constructor 1 and then create an object wfB using the copy constructor. Finally, I add these to classes and create a new object wfC from their addition. To make this optimal (at least according to my understanding) I use std::move().

The output of the above main program is:

wf: constructor 1
wf: copy constructor
wf: classes addition
wf: constructor 1
wf: move constructor
wf: destructor
nx = 8
ny = 8
nf = 4
wfSize = 256
(0,0) (2,0) (4,0) (6,0) (8,0) 
wf: destructor
wf: destructor
wf: destructor

this result is what I expected. i.e. there are two calls to contructor 1, one for the obect wfA and one for the object wfC, I have one call to the copy constructor for object wfB, and then the move constructor is called to move the semantics of rvalue(temporary) object wfA + wfB to wfC. Finally, the 4 destructors account for wfA, wfB, wfC and the temporay object wfA + wfB (right?)!

I think so far so good! (correct me if it is not totally okay!)

Now, I throw away the std::move() statement, so my main program becomes:

int main(int argc, char * argv[]){

    wf wfA(8,8,4);
    for(unsigned int i=0; i<wfA.wfSize; i++){
        float fl = (float)(i);
        wfA.val[i] = fcomp(fl,0.0);
    }

    wf wfB(wfA);

    wf wfC = wfA + wfB;

    wfC.dispInfo();

    for(unsigned int i=0; i<5; i++) 
        std::cout << wfC.val[i] << " ";

    std::cout << std::endl;
    return 0;
}

the output now becomes:

wf: constructor 1
wf: copy constructor
wf: classes addition
wf: constructor 1
nx = 8
ny = 8
nf = 4
wfSize = 256
(0,0) (2,0) (4,0) (6,0) (8,0) 
wf: destructor
wf: destructor
wf: destructor

I see that the move constructor has not been called now so I expected that I would have a call to the copy assignement operator, or even the copy constuctor since the operator+ returns the object by value, right?.

But, I cannot see any such call...

So the question is: What is really happening now in my code?

Is there any copy happening but cannot see it, or maybe the compiler sees what I am trying to do and thus optimize it in advance by creating the wfC using 1 constructor call?

Also, which is the optimal way to do this operation, using std::move() or without using it?

Note: I compiled the code using -O3 and -O0 but the output did not change.

Andreas Hadjigeorgiou
  • 45
  • 1
  • 8
  • I don't think the question should be marked as duplicate because I see about `copy elision` after trying to understand the copy constructor... similarly one can follow my question and go to copy elision, where he would probably not if didn't see my post.. I think. But, ok anyway! – Andreas Hadjigeorgiou Jun 04 '20 at 10:20
  • 1
    Compiling your program with `clang++ -Wpessimizing-move ...` gives: "_warning: moving a temporary object prevents copy elision_" and "_remove `std::move` call here: `wf wfC = std::move(wfA + wfB);`_". Strangely enough, `g++` doesn't warn about it even though the effect is just as bad there. – Ted Lyngmo Jun 04 '20 at 10:27
  • I see, thanks @TedLyngmo ! So, there is a trade of now between using std::move() or not. – Andreas Hadjigeorgiou Jun 04 '20 at 10:45
  • so when the compiler does `copy elision` the object is constructed in the place of the returned object, and this is the most efficient way to do it, right? – Andreas Hadjigeorgiou Jun 04 '20 at 10:48
  • 1
    Yes, you shouldn't just blindly add `std::move` because it might result in a pessimization. For temporary objects like the one `wfA + wfB` creates, don't `std::move`. Also, add a lot of warning flags. This case was not caught by `g++` so if possible, compile with more than one compiler to catch as much as possible. I use both `g++` and `clang++` when testing. – Ted Lyngmo Jun 04 '20 at 10:48
  • But if the compiler does not do a `copy elision` I will have an extra copy operation. And in this case `std::move` would be the fastest way. So, nasty!! – Andreas Hadjigeorgiou Jun 04 '20 at 10:51
  • 1
    No, if the compiler wouldn't do copy elision (or RVO) it would result in a move, not a copy, so the `std::move` is not needed in any case. – Ted Lyngmo Jun 04 '20 at 10:55
  • 1
    Also, C++17 mandates [copy elision and RVO](https://en.cppreference.com/w/cpp/language/copy_elision). – Yksisarvinen Jun 04 '20 at 10:55
  • 1
    Okay thanks a lot @TedLyngmo ! and so would use my user-defined `move constructor`... (or maybe the `move assignment operator` ?) – Andreas Hadjigeorgiou Jun 04 '20 at 11:04
  • This doesn't address the question, but the title slurs over an important bit of terminology: you don't return a **class**, you return an **object**. Don't muddle the two. – Pete Becker Jun 04 '20 at 12:35
  • So what is the important difference if possible explain here with a comment? – Andreas Hadjigeorgiou Jun 04 '20 at 12:55

0 Answers0