Possible to get a warning when using std::move on a const?

[Peter87]

Some people recommend using const as much as possible but one of the things that bothers me with this is that sometimes I might want to move from a variable, so it cannot be const. Fine, so I make it non-const, right?

My fear is that I will come back later and notice the variable is not const. My const obsession kicks in and I habitually add const. The code compiles and I think everything is fine, except that I have just disabled the move.

I think it would be very useful if compilers could generate a warning when using std::move on a constant. I understand that it could probably generate a few false positives in templated code but I would be more than happy if it only warned on types that were not dependent on template arguments.

Is anyone aware of a compiler flag or other tool that can warn about this? I'm also interested to hear if anyone has any other ideas on how to deal with this const vs. move issue. Should I not use const? Is there a way to avoid std::move altogether? That sort of thing...

[TheIdeasMan]

https://clang.llvm.org/extra/clang-tidy/checks/performance-move-const-arg.html

I found this for clang.


Regards :+)

[mbozzi]

I'm also interested to hear if anyone has any other ideas on how to deal with this const vs. move issue:

- std::move(x) only results in a move if the object x has a move constructor defined. Otherwise it causes a copy.
- std::move(x) where x is const only results in a move if the object x has a const move constructor defined - the constructor accepting a const rvalue reference A(A const&& old);

It's not always wrong to move const things - this is a product of the meaning of const as "logically constant" as opposed to "bit-wise constant":
https://isocpp.org/wiki/faq/const-correctness#logical-vs-physical-const

Is there a way to avoid std::move altogether?
Not that I know of in general, but with awareness of (N)RVO explicit moves are quite rare. Of course your use case might be different.

You could write your own move function that fails when it doesn't detect a suitable move constructor when called with a const value. Maybe it's possible to have the compiler emit a warning instead of an error?

[JLBorges]

> My fear is that I will come back later and notice the variable is not const.
> My const obsession kicks in and I habitually add const.

Want to move from an object that would otherwise have been logically const?
I suspect that this situation would be extremely rare in real life code; if I do encounter it, this is what I'll do:

/*const*/ std::string str = " ... " ; // not const, since we want to move from it later

[Peter87]

TheIdeasMan wrote:

I found this for clang.

Thank you. It seems to be exactly what I was looking for. I'm not normally using the llvm toolchain but I could probably get myself to run clang-tidy once in a while to look for these (and other) problems in my code.

mbozzi wrote:

It's not always wrong to move const things

You mean like casting away the constness? I'm not sure I want to go down that direction. I'm afraid I that I might accidentally end up moving from something that is truly const.

mbozzi wrote:

You could write your own move function that fails when called with a const value.

Yeah, that might actually be the best solutions because it requires no extra tools.

template <typename T>
decltype(auto) force_move(T&& value)
{
	static_assert(!std::is_const_v<std::remove_reference_t<T>>, "Move can't be forced on const object.");
	return std::move(value);
}

The only disadvantage is that if you do use a tool it might silence other warnings that the tool gives you. The page that TheIdeasMan linked to says that clang-tidy also warns about using std::move on trivially-copyable types and when the result is used as a const reference. Adding another static_assert to check for trivially-copyable types is easy but I don't think there is a way to guard against the return value being used incorrectly. This is not a big problem in my opinion but still something worth taking into consideration.

JLBorges wrote:

Want to move from an object that would otherwise have been logically const?
I suspect that this situation would be extremely rare in real life code;

My main concern is with local variables. Some people, like Jason Turner (https://youtu.be/uzF4u9KgUWI?t=7m48s), recommend using const essentially everywhere. I'm not so used to using const for local variables (except for compile-time constants) so maybe I'm overinterpreting what they are saying and use const in too many places.

Let me give an example. Say that I want to load objects of type X from files and store them in a vector. To construct an X object I need to pass it a name and a filepath. I want to use the filename as the name and since I no longer will be needing the filename I can just as well move it, so I end up with the following code:

const std::vector<std::string> filenames = dir.files();
std::vector<X> xs;

for (const std::string& filename : filenames)
{
	const std::string filepath = dir.str() + '/' + filename;
	xs.emplace_back(std::move(filename), filepath);
}

The problem here is that the filename does not get moved because it is const. I would probably have realized this when adding std::move so the code would probably have looked more like this:

std::vector<std::string> filenames = dir.files();
std::vector<X> xs;

for (std::string& filename : filenames)
{
	const std::string filepath = dir.str() + '/' + filename;
	xs.emplace_back(std::move(filename), filepath);
}

But now I have the concern that I described earlier. It is not immediately obvious why filenames and filename are not const so I might accidentally come back and "correct" the code later. If I had used auto& instead of std::string& for the filename the risk is even higher because all it takes is that I change the first line.

Am I doing something wrong here or am I just careless if I add const without thinking long and hard about the consequences first? Before we had move semantics it used to be safe to just throw const at everything. If it didn't work you just got an error message telling you where the problem was so that you could make a decision about whether or not const was appropriate right away.

[JLBorges]

> Say that I want to load objects of type X from files and store them in a vector. To construct
> an X object I need to pass it a name and a filepath. I want to use the filename as the name
> and since I no longer will be needing the filename I can just as well move it

I would just directly use rvalues instead of named objects.

// std::vector<std::string> filenames = dir.files();
std::vector<X> xs;

for( auto&& filename : dir.files() )
{
    // const std::string filepath = dir.str() + '/' + filename;
    xs.emplace_back( std::move(filename), dir.str() + '/' + filename ); // move both strings
}

[mbozzi]

You mean like casting away the constness? I'm not sure I want to go down that direction. I'm afraid I that I might accidentally end up moving from something that is truly const.

const is just a promise to the outside world that the visible state of the object won't change. It doesn't mean that internal state doesn't change; we have mutable to express this.

There are some (very rare) situations where const objects contain expensive-to-copy, cheap-to-move mutable state. Then we could write a move constructor accepting a rvalue reference to const that makes sense -- just a corner case.

[helios]

Peter87: Perhaps as a reminder that something that must never be const you could #define nonconst .

mbozzi: How do you define "internal state" and "visible state"? It's possible to define a valid class such that

class A{
    //...
public:
    //Note: f() is deterministic (it performs no I/O and doesn't use shared data)
    T f() const;
    void g() const;
};

A a;
auto old = a.f();
a.g();
assert(a.f() != old);

That aside, I don't understand how you would define a move-constructor-from-const without using casting hacks to get around the compiler not letting you modify the passed object.

[mbozzi]

I don't understand how you would define a move-constructor-from-const without using casting hacks to get around the compiler not letting you modify the passed object.

You're misunderstanding the meaning of const:

cppreference wrote:

const object - an object whose type is const-qualified, or a non-mutable subobject of a const object. Such an object cannot be modified [...].

Example:

#include <iostream>
#include <map>

struct fibonacci_fn_t {
  fibonacci_fn_t() = default;
  fibonacci_fn_t(fibonacci_fn_t const &&other)
    : memo(std::move(other.memo)) {
    std::cout << "move-construction from rvalue-reference to const\n";
  }

  // Nth term of the fibonacci sequence, zero-indexed.

  // A method that computes terms of the Fibonacci sequence is logically
  // constant. The use of internal state for a memo is just an optimization,
  // it's an implementation detail not relevant to the caller.

  // We shouldn't sacrifice the const qualification on the member function just
  // to allow us to implement some optimization. Indeed, that's a breaking
  // change: the const qualification is part of the interface!
  int operator()(int n) const { return fib_impl(n); }

private:
  int fib_impl(int n) const
  { return memo[n] != 0? memo[n]: memo[n] = fib_impl(n - 1) + fib_impl(n - 2); }

  // first 2 terms of the fibonacci sequence: f_0 = 1, f_1 = 1.
  // NOTE: expensive-to-copy, cheap-to-move, and mutable.
  mutable std::map<int, int> memo{{0, 1}, {1, 1}};
};

int main() {
  fibonacci_fn_t const old_fib{};
  std::cout << "old_fib(40) = " << old_fib(40) << '\n';
  fibonacci_fn_t const new_fib(std::move(old_fib));
  std::cout << "new_fib(40) = " << new_fib(40) << '\n';
}

Output:

old_fib(40) = 165580141
move-construction from rvalue-reference to const
new_fib(40) = 165580141

Live demo:
http://coliru.stacked-crooked.com/a/9db616189ab3f001

[helios]

No, I just didn't know about mutable.

The issue I have with that is that you're kind of defeating the point of const. All data members of fibonacci_fn_t are mutable, so it's the same as nullifying constness for that type. const and non-const member functions have equal power to change the state of the object.

[mbozzi]

All data members of fibonacci_fn_t are mutable, so it's the same as nullifying constness for that type.

If you added a non-mutable data member nothing changes - the move constructor would just copy that member if necessary. No problem. Still, time is saved by moving as much as possible.

Const and non-const member functions have equal power to change the state of the object.

This is always the case, thanks to const_cast and mutable. Of course it's bad practice to make mutable things visible externally through const members.

https://isocpp.org/wiki/faq/const-correctness#logical-vs-physical-const

[helios]

If you added a non-mutable data member nothing changes - the move constructor would just copy that member if necessary. No problem. Still, time is saved by moving as much as possible.

What I mean is that there's no difference between a fibonacci_fn_t and a const fibonacci_fn_t, or a fibonacci_fn_t & and a const fibonacci_fn_t &.

This is always the case, thanks to const_cast

What do you mean? Casting the constness away? Yeah, you can do that if you don't care about undefined behavior.

https://isocpp.org/wiki/faq/const-correctness#logical-vs-physical-const

I see lots of assertions, but no justifications. I don't necessarily disagree, but I'd like to see at least some reasoning for why it should be one way rather than the other.

[TheIdeasMan]

helios wrote:

What I mean is that there's no difference between a fibonacci_fn_t and a const fibonacci_fn_t, or a fibonacci_fn_t & and a const fibonacci_fn_t &.

By using an rvalue reference, or a forwarding reference, it means that moving or forwarding will work for lvalue, rvalue, non-const, and const types.

helios wrote:

I see lots of assertions, but no justifications. I don't necessarily disagree, but I'd like to see at least some reasoning for why it should be one way rather than the other.

The very next section:
https://isocpp.org/wiki/faq/const-correctness#mutable-data-members

I think the idea of mutable is where one has a class where one of the members stores a cached value to improve the performance of the next look up because it's an expensive operation that may only need to be recalculated rarely, but looked up often. By using mutable on this member, const functions can still change it's value.

Kate Gregory has a video about this, sorry I don't have time to search for it right now - Lunch time is nearly over :+)

Regards

[mbozzi]

What do you mean? Casting the constness away? Yeah, you can do that if you don't care about undefined behavior.

You can cast away const when the object referred to by *this is non-const. When that assumption holds it gives const methods the ability to change the state of *this.

I'd like to see at least some reasoning for why it should be one way rather than the other.

If we allow the interface (i.e., of operator()()) to reflect a detail of the particular implementation being used (by omitting const), we're sharing more information than is required. Not sharing information about the object's physical state is a kind of information hiding; we gain flexibility that way.

Why should the caller care whether or not this fibonacci function caches data?
Isn't it more important to inform the caller that the fibonacci function's behavior doesn't change when you compute it? The code's clearer that way, at least.

[helios]

If it's just a matter of data hiding, just make everything non-const, or everything const with only mutable members. That way you're revealing the least possible amount of information about the implementation.

What's the point of const-correctness if constness only reflects the state of some hypothetical property that doesn't necessarily correspond to anything the program actually does? Is it even possible to talk about "correctness" if constness is orthogonal to program state?

[mbozzi]

If it's just a matter of data hiding, just make everything non-const, or everything const with only mutable members. That way you're revealing the least possible amount of information about the implementation.

When I used the word "implementation" above I meant the implementation of the interface involving a particular class. Information hiding offers benefits to the caller only when we hide details about the guts of an abstraction (e.g., a class) behind a nice interface (e.g., the class's public members). If a caller depends on those details (implementation details), the caller's code has to be checked and possibly changed whenever the implementation changes.

But never or always using const hides interface details, not implementation details. That makes const (or the lack of it) meaningless, depriving us of an opportunity to provide semantic information about the code to the caller. That's an attempt to hide the wrong information, like changing all the member function names to meaningless words.

See: https://en.wikipedia.org/wiki/Information_hiding

[helios]

But never or always using const hides interface details, not implementation details.

I would argue that it's both removing interface details, and hiding implementation details. It doesn't really make sense to talk about hiding an interface; if you hide it's no longer an interface.

And yes, if you can't tell if a function is going to modify its parameters or not, you are hiding implementation details. For example

float average_of_frequency_domain(const std::vector<float> &time_domain);
float average_of_frequency_domain(std::vector<float> &time_domain);

One of these functions promises not to modify its input, while the other doesn't. Whether the function needs to modify its input is an implementation detail, which the second function may be hiding (depending on context).

That makes const (or the lack of it) meaningless, depriving us of an opportunity to provide semantic information about the code to the caller.

Using mutable makes const meaningless anyway. In your example above operator()() claims to be const, but this is a complete lie. Calling the function multiple times with the same parameters will produce observably different behavior, even though there were no changes to state outside the object.
Are you saying that it's not lying, or that lying is better than saying nothing?

[TheIdeasMan]

In the example where a class has for example a cache value, it is the one which is mutable, and it is private. It means the other member functions can still be const, we are not telling lies about our interface. By that I mean the accessor function that returns the cached value can still be const, even though it might call a function that recalculates the mutable cached value.

So it's not about the constness of function arguments, and we are not saying one should use mutable everywhere. Like a lot of things there are specific places where it should be used.

[helios]

Sorry, but I don't see how that's not lying. A pure function accepting const and value parameters should not behave differently when passed the same parameters. If it does then either it uses external data (it's not pure) or it modified its by-reference parameters (they're not const).
I don't care if members are mutable; as you say, they're private so they're not part of the interface. If the behavior is observably different then this' constness is a lie.

I'm not saying anyone is arguing for using mutable everywhere. I'm saying using it anywhere makes const meaningless, as it disconnects it from anything the program is actually doing.
EDIT: I suppose an exception could be public mutable members.

EDIT 2: I thought of another case where mutable would be valid without making constness a lie:

class Foo{
    mutable std::mutex m;
    int data;
public:
    int get_data() const{
        std::lock_guard<std::mutex> lg(this->m);
        return this->data;
    }
    //...
};

[mbozzi]

One of these functions promises not to modify its input, while the other doesn't.

There is no such promise if we decided not to use const meaningfully in the first place.

A pure function accepting const and value parameters should not behave differently when passed the same parameters.

How does it behave differently? It's maybe faster the second time? IMO, that behavior shouldn't be part of the contract; it shouldn't dictate the method's interface. As long as the pre- and post-conditions are upheld, an implementation can satisfy them however it wants.