[PATCH] New warning -Wpessimizing-move to catch when removing calls to std::move prevent optimization

[Richard Trieu]

================
Comment at: lib/Sema/SemaInit.cpp:3245-3246
@@ +3244,4 @@
+    return;
+  if (SemaRef.Diags.isIgnored(diag::warn_pessimizing_move_on_initialization,
+                              InitExpr->getLocStart()))
+    return;
----------------
rsmith wrote:
> This check is not especially cheap, and the code below will be cheap in most cases; it's probably better to omit this.
isIgnored check removed.

================
Comment at: lib/Sema/SemaInit.cpp:3249
@@ +3248,3 @@
+
+  const CallExpr *CE = dyn_cast<CallExpr>(InitExpr);
+  if (!CE || CE->getNumArgs() != 1)
----------------
rsmith wrote:
> Maybe `IgnoreParens` before the cast?
More parens removed.

================
Comment at: lib/Sema/SemaInit.cpp:3259
@@ +3258,3 @@
+
+  const Expr *Arg = CE->getArg(0)->IgnoreImplicit();
+  if (!Arg->isRValue() || !Arg->getType()->isRecordType())
----------------
rsmith wrote:
> Maybe `IgnoreParens` here too.
More parens removed.  Plus test cases for parens.

================
Comment at: lib/Sema/SemaInit.cpp:3392-3393
@@ -3361,1 +3391,4 @@
+
+  if (Args.size() == 1)
+    CheckMoveOnConstruction(S, Args[0]);
 }
----------------
rsmith wrote:
> You should defer the diagnostic until we perform the `InitializationSequence`. We might select a different initialization sequence or overload candidate and not actually call this constructor.
Moved check to after InitializationSequence in PerformCopyInitiailization.

================
Comment at: lib/Sema/SemaStmt.cpp:3049
@@ -3048,1 +3048,3 @@
 
+static void CheckMoveOnReturn(Sema &SemaRef, const Expr *ReturnExpr,
+                              const FunctionDecl *FD) {
----------------
rsmith wrote:
> This looks familiar. Can some commonality be factored out here?
> 
> Or, better, can you implement both checks in a single place? When performing the initialization sequence, you can ask the initialized entity whether it's the returned object in a return statement.
Moved to a single checking function in SemaInit

================
Comment at: lib/Sema/SemaStmt.cpp:3080-3081
@@ +3079,4 @@
+
+  if (!SemaRef.Context.hasSameUnqualifiedType(ReturnType, VD->getType()))
+    return;
+
----------------
rsmith wrote:
> If the types don't match, you aren't likely to have a pessimizing move (elision would only occur if your return type's constructor took its argument by value), but perhaps we should still produce some kind of "redundant move" warning. (Per http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579, you get an implicit move no matter what the types are.)
Added new warning for redundant moves on return.

http://reviews.llvm.org/D7633

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/