[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 accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/clang/Sema/Sema.h:3827
   bool isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
-                              bool AllowParamOrMoveConstructible);
+                              CopyElisionSemanticsKind CESK);
 
----------------
Quuxplusone wrote:
> Q: Is it appropriate for me to be changing the signature of this public method at all? I don't know libclang's rules regarding this kind of change.
For reference, changes here are fine. We provide no API or ABI stability guarantees for our C++ ABI; libclang provides an ABI-stable C ABI layered on top of our internal API for this reason.


================
Comment at: include/clang/Sema/Sema.h:3806
     CES_Default = (CES_AllowParameters | CES_AllowDifferentTypes),
+    CES_AsIfByStdMove = (CES_AllowParameters | CES_AllowDifferentTypes | CES_AllowExceptionVariables),
   };
----------------
Please wrap to 80 columns.


================
Comment at: lib/Sema/SemaStmt.cpp:3075-3076
+      } else {
+        // The most common case for this is returning T from a function that
+        // returns Expected<T>. This is totally fine in a post-CWG1579 world,
+        // but was not fine before.
----------------
I've seen this a lot more for returning `unique_ptr<Derived>` from a function with return type `unique_ptr<Base>`. Maybe just give this `Expected<T>` case as an example rather than saying it's the most common example?


Repository:
  rC Clang

https://reviews.llvm.org/D43322