2

I have a state class which has a move assignment/constructor. The copy assingment/constructor are set to delete.

I am confused why in the following function (which returns a state object) compiles and runs fine like this:

state propagator::PROPAGATE(date & TargetDate)
{
    jmethodID jmid_PROPAGATE = ENV->GetMethodID(this->jcls_PROPAGATOR, "propagate", "(path/to/date;)path/to/state;");
    jobject jobj_PROPAGATED_STATE = ENV->CallObjectMethod(this->jobj_PROPAGATOR, jmid_PROPAGATE, TargetDate.get_DATE_JOBJECT());

    state PROPAGATED_STATE(this->ENV);
    PROPAGATED_STATE.set_STATE_JOBJECT(jobj_PROPAGATED_STATE);
    return PROPAGATED_STATE;

    //state * PROPAGATED_STATE = new state(ENV);
    //PROPAGATED_STATE->set_STATE_JOBJECT(jobj_PROPAGATED_STATE);
    //return *PROPAGATED_STATE;
}

but complains that copy constructor has been deleted when I try this:

state propagator::PROPAGATE(date & TargetDate)
{
    jmethodID jmid_PROPAGATE = ENV->GetMethodID(this->jcls_PROPAGATOR, "propagate", "(path/to/date;)path/to/state;");
    jobject jobj_PROPAGATED_STATE = ENV->CallObjectMethod(this->jobj_PROPAGATOR, jmid_PROPAGATE, TargetDate.get_DATE_JOBJECT());

    //state PROPAGATED_STATE(this->ENV);
    //PROPAGATED_STATE.set_STATE_JOBJECT(jobj_PROPAGATED_STATE);
    //return PROPAGATED_STATE;

    state * PROPAGATED_STATE = new state(ENV);
    PROPAGATED_STATE->set_STATE_JOBJECT(jobj_PROPAGATED_STATE);
    return *PROPAGATED_STATE;
}

Compiler output:

error: use of deleted function ‘state::state(const state&)’
Ddor
  • 347
  • 1
  • 12
  • Why do you even create the object on the heap? Thats a memory leak. – tkausl Nov 07 '18 at 09:36
  • I have deleted some of the code to make the question/code easier to read, but I am actually setting the pointer to the heap allocated object to a static member variable in the `state` class which is then deleted within the class destructor. – Ddor Nov 07 '18 at 09:40
  • Because the first one moves and the second one would copy. – molbdnilo Nov 07 '18 at 09:40

1 Answers1

2

Modern compilers are clever enough to do the RVO (What are copy elision and return value optimization?)

state propagator::PROPAGATE(date & TargetDate)
{
    jmethodID jmid_PROPAGATE = ENV->GetMethodID(this->jcls_PROPAGATOR, "propagate", "(path/to/date;)path/to/state;");
    jobject jobj_PROPAGATED_STATE = ENV->CallObjectMethod(this->jobj_PROPAGATOR, jmid_PROPAGATE, TargetDate.get_DATE_JOBJECT());

    state PROPAGATED_STATE(this->ENV);
    PROPAGATED_STATE.set_STATE_JOBJECT(jobj_PROPAGATED_STATE);
    return PROPAGATED_STATE;
}

That's why here we return actually the object which was created (compiler can just create it in place for function's return value, to avoid copying).

But in second variant you are trying to create object in the stack from object in the HEAP, and RVO or move can't be used here. That's why it is trying to perform the deleted copy.

state propagator::PROPAGATE(date & TargetDate)
{
    jmethodID jmid_PROPAGATE = ENV->GetMethodID(this->jcls_PROPAGATOR, "propagate", "(path/to/date;)path/to/state;");
    jobject jobj_PROPAGATED_STATE = ENV->CallObjectMethod(this->jobj_PROPAGATOR, jmid_PROPAGATE, TargetDate.get_DATE_JOBJECT());

    state * PROPAGATED_STATE = new state(ENV);
    PROPAGATED_STATE->set_STATE_JOBJECT(jobj_PROPAGATED_STATE);
    return *PROPAGATED_STATE;
}

Also you are leaking memory by discarding pointer to heap, where you've created an object.

Arkady
  • 2,084
  • 3
  • 27
  • 48
  • 1
    Hi Arkady, thanks a lot for your comment - great explanation. So essentially it is because RVO doesn't work for objects on the heap -got it! – Ddor Nov 07 '18 at 09:48
  • "So essentially it is because RVO doesn't work for objects on the heap" It obviously doesn't work for objects on the heap, it can't just steal an objects guts. The only reason it works for local objects is because they go out of scope and get destroyed anyway. You'd need to `std::move` yourself if you want it to move-out the object. – tkausl Nov 07 '18 at 09:56
  • It is NRVO not RVO in OP's case. – Jarod42 Nov 07 '18 at 09:56
  • and pre-C++17, compiler should emit same error in both cases (even if it would apply (N)RVO). Since C++17, we have guarantied elision in some case. – Jarod42 Nov 07 '18 at 09:59
  • @Jarod42 as far as I know, if move assignment was defined, there shouldn't be a problem with first case, even if elision is not guaranteed – Arkady Nov 07 '18 at 17:42
  • @Arkady: Indeed. – Jarod42 Nov 07 '18 at 18:07