Should I make my local variables const or movable?

Question

My default behaviour for any objects in local scopes is to make it const. E.g.:

auto const cake = bake_cake(arguments);

I try to have as little non-functional code as I can as this increases readability (and offers some optimisation opportunities for the compiler). So it is logical to also reflect this in the type system.

However, with move semantics, this creates the problem: what if my cake is hard or impossible to copy and I want to pass it out after I'm done with it? E.g.:

if (tastes_fine(cake)) {
  return serve_dish(cake);
}

As I understand copy elision rules it's not guaranteed that the cake copy will be elided (but I'm not sure on this).

So, I'd have to move cake out:

return serve_dish(std::move(cake)); // this will not work as intended

But that std::move will do nothing useful, as it (correctly) will not cast Cake const& to Cake&&. Even though the lifetime of the object is very near its end. We cannot steal resources from something we promised not to change. But this will weaken const-correctness.

So, how can I have my cake and eat it too?

(i.e. how can I have const-correctness and also benefit from move semantics.)

Answer 1

I believe it's not possible to move from a const object, at least with a standard move constructor and non-mutable members. However, it is possible to have a const automatic local object and apply copy elision (namely NRVO) for it. In your case, you can rewrite your original function as follows:

Cake helper(arguments)
{
   const auto cake = bake_cake(arguments);
   ...  // original code with const cake
   return cake;  // NRVO 
}

Then, in your original function, you can just call:

return serve_dish(helper(arguments));

Since the object returned by helper is already a non-const rvalue, it may be moved-from (which may be, again, elided, if applicable).

Here is a live-demo that demonstrates this approach. Note that there are no copy/move constructors called in the generated assembly.

Answer 2

It seems to me, that if you want to move, than it will be "const correct" to not declare it const, because you will(!) change it. It's ideological contradiction. You cannot move something and leave in place at the same time. You mean, that object will be const for a part of time, in some scope. In this case, you can declare const reference to it, but it seems to me, that this will complicate the code and will add no safety. Even vice versa, if you accidentally use the const reference to object after std::move() there will be problems, despite it will look like work with const object.

Answer 3

Make them movable if you can.

It's time to change your "default behaviour" as it's anachronistic.

If move semantics were built into the language from inception then making automatic variables const would have quickly become established as poor programming practice.

const was never intended to be used for micro-optimisations. Micro-optimisations are best left to the compiler. const exists primarily for member variables and member functions. It's also helped clean up the language a little: e.g. "foo" is a const char[4] type whereas in C it's a char[4] type with the curious understanding that you're not allowed to modify the contents.

Now (since C++11) const for automatic variables can actually be harmful as you observe, the time has come to stop this practice. The same can be said for const parameter by-value types. Your code would be less verbose too.

Personally I prefer immutable objects to const objects.

Answer 4

You should indeed continue to make your variables const as that is good practice (called const-correctness) and it also helps when reasoning about code - even while creating it. A const object cannot be moved from - this is a good thing - if you move from an object you are almost always modifying it to a large degree or at least that is implied (since basically a move implies stealing the resources owned by the original object) !

From the core guidelines:

You can’t have a race condition on a constant. It is easier to reason about a program when many of the objects cannot change their values. Interfaces that promises “no change” of objects passed as arguments greatly increase readability.

and in particular this guideline :

Con.4: Use const to define objects with values that do not change after construction

---

Moving on to the next, main part of the question:

Is there a solution that does not exploit NRVO?

If by NRVO you take to include guaranteed copy elision, then not really, or yes and no at the same. This is somewhat complicated. Trying to move the return value out of a return by value function doesn't necessarily do what you think or want it to. Also, a "no copy" is always better than a move performance-wise. Therefore, instead you should try to let the compiler do it's magic and rely in particular on guaranteed copy elision (since you use c++17). If you have what I would call a complex scenario where elision is not possible: you can then use a move combined with guaranteed copy elision/NRVO, so as to avoid a full copy.

So the answer to that question is something like: if you object is already declared as const, then you can almost always rely on copy-elision/return by value directly, so use that. Otherwise you have some other scenario and then use discretion as to the best approach - in rare cases a move could be in order(meaning it's combined with copy-elision).

Example of 'complex' scenario:

std::string f() {
  std::string res("res");
  return res.insert(0, "more: ");//'complex scenario': a reference gets returned here will usually mean a copy is invoked here.
}

Superior way to 'fix' is to use copy-elision i.e.:

return res;//just return res as we already had that thus avoiding copy altogether - it's possible that we can't use this solution for more *hairy/complex* scenarios.

Inferior way to 'fix' in this example would be;

return std::move(res.insert(0, "more: "));

Answer 5

A limited workaround would be const move constructor:

class Cake
{
public:
    Cake(/**/) : resource(acquire_resource()) {}
    ~Cake() { if (owning) release_resource(resource); }

    Cake(const Cake& rhs) : resource(rhs.owning ? copy_resource(rhs.resource) : nullptr) {}
    // Cake(Cake&& rhs) // not needed, but same as const version should be ok.
    Cake(const Cake&& rhs) : resource(rhs.resource) { rhs.owning = false; }

    Cake& operator=(const Cake& rhs) {
        if (this == &rhs) return *this;
        if (owning) release_resource(resource);
        resource = rhs.owning ? copy_resource(rhs.resource) : nullptr;
        owning = rhs.owning;
    }
    // Cake& operator=(Cake&& rhs) // not needed, but same as const version should be ok.
    Cake& operator=(const Cake&& rhs) {
        if (this == &rhs) return *this;
        if (owning) release_resource(resource);
        resource = rhs.resource;
        owning = rhs.owning;
        rhs.owning = false;
    }
    // ...

private:
    Resource* resource = nullptr;
    // ...
    mutable bool owning = true;
};

• Require extra mutable member.
• not compatible with std containers which will do copy instead of move (providing non const version will leverage copy in non const usage)
• usage after move should be considered (we should be in valid state, normally). Either provide owning getter, or "protect" appropriate methods with owning check.

I would personally just drop the const when move is used.