[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

In https://reviews.llvm.org/D43322#1017190, @lebedev.ri wrote:

> Somewhat related thing i have noticed: https://godbolt.org/g/ow58JV


IIUC, you're asking whether it would be possible to detect instances of

  return take(mysharedptr);

and rewrite them into

  return take(std::move(mysharedptr));

(Notice that the "return" context is relatively important, because if `mysharedptr` is used again after this use, then the move-from is unsafe and should not be suggested. Of course it's always possible for `mysharedptr` to be used again after a `return` or `throw` as well, e.g. during stack unwinding/destructors; but there's precedent there for C++ saying we'll take that chance.)

If you start treating names as rvalues no matter where they appear in larger expressions, then you run into trouble with

  std::string hello = ...;
  std::string &hello2 = hello;
  return concatenate(hello, hello2);

where the first use of `hello` on this line moves-out-of it, and then the second use reads the moved-from value. My best guess is that things like that occur frequently enough in real-world code that (A) the Standard can't specify move behavior there because it would quietly break code, and (B) a Clang diagnostic would produce mostly false positives (in the sense that accepting the fixits would quietly break the user's code).  I admit it would be interesting to write the code and find out empirically.

Whereas in this specific limited case of `return x;` and `throw x;`, programmers are already trained to expect a move, and there are no (new) aliasing issues.

> Where though, as clang-tidy check?

My experience with this patch suggests that when the thing you need to do involves "run a hypothetical overload-resolution," it is basically impossible to do with clang-tidy, and relatively easy to do in clang proper. (I tried writing this check into clang-tidy first, and couldn't figure out how to do the overload resolution part. Whereas it was painless in clang, especially with `-Wpessimizing-move` to copy off of.)


Repository:
  rC Clang

https://reviews.llvm.org/D43322