ValueFlow: avoid various unnecessary copies by firewave · Pull Request #7583 · cppcheck-opensource/cppcheck

It matches the pattern we currently use all over the code.

I havent really seen this pattern in the code. We usually use a const ref when we only want to read the values.

Although the explicit rvalue move makes the code more explicit it also uglifies it.

Using a non-const ref makes the code harder to read. It looks like the intention is not to read the value but to modify it non-locally. The rvalue makes the intention clear that it wants to move the value, so it works just like a non-reference where mutations are local since it has unique ownership.

• const T& x = y shows it will only read x and y and there is no mutations.
• T x = y shows it will not mutate y and any mutations to x are local
• T& x = y shows it will mutate x and y, so mutations are not local.
• T&& x = std::move(y) shows x has unique ownership of y and any mutations are local since y will not be used later. So you can essentially read it as T x = y.

Writing T& x = y when you are really doing T&& x = std::move(y) requires more time to read the scope of everything to understand what is really being mutated which is why it makes it harder to read.