Add our own std::move
that fails to compile when used on a const object
#957
Labels
std::move
that fails to compile when used on a const object
#957
It's really easy to accidentally try to move an object that can't actually be moved because it's const. For example:
Seems legit at a glance, right? Nope, the
std::move(capturedValue)
does nothing at all, becausecapturedValue
is const, despite the wordconst
not appearing anywhere. So ifcallSomething
has two overloads, one that takes aconst std::string&
and the other astd::string&&
, this will end up calling theconst std::string&
one. And so we'll get a copy, which can have pretty severe performance implications in some cases.The solution is pretty subtle:
The only change in this new version, which will work as expected, is that the lambda has been declared
mutable
.Unreal Engine has a function called
MoveTemp
that helps avoid this class of problems. It's just likestd::move
, except it also uses astatic_assert
to cause a compiler error if given a const reference. We should add something similar to cesium-native (perhapsCesiumUtility::move
?), and use it everywhere. There's a chance it will immediately flag some copies we didn't realize we were making.I suspect something like clang-tidy can flag this, too, so that's another option.
The text was updated successfully, but these errors were encountered: