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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:23
+def warn_return_std_move : Warning<
+  "adding 'std::move' here would select a better conversion sequence">,
+  InGroup<ReturnStdMove>, DefaultIgnore;
----------------
Can we say something like "local variable '%0' will be copied despite being %select{returned|thrown}1 by name; call 'std::move' explicitly to avoid the copy"? (Would that be accurate, given the implementation of the warning?)

Ideally, we'd move the "call 'std::move' explicitly" hint to a separate note diagnostic and include a FixItHint on that diagnostic to insert the call to `std::move`.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:27
+def warn_return_std_move_in_cxx11 : Warning<
+  "adding 'std::move' here would have selected a better conversion sequence prior to CWG 1579">,
+  InGroup<ReturnStdMoveInCXX11>, DefaultIgnore;
----------------
I think the wording of this diagnostic could be improved too. It's very language-lawyer-friendly right now. Here's one possible starting point for alternative wording (though I think maybe we can do better):

"ISO C++11, prior to the resolution of defect reports, would have made a copy of local variable '%0' despite it being returned by name due to a type mismatch with the return value%diff{ ($ vs $)|}1, 2"

... plus a "note: call 'std::move' explicitly to avoid a copy when using older compilers" or similar, with a fixit.


Repository:
  rC Clang

https://reviews.llvm.org/D43322