[PATCH] New warning -Wpessimizing-move to catch when removing calls to std::move prevent optimization
Richard Smith
richard at metafoo.co.uk
Wed Apr 22 20:25:27 PDT 2015
================
Comment at: lib/Sema/SemaInit.cpp:7346
@@ +7345,3 @@
+ QualType DestType = InitExpr->getType();
+ if (DestType->isReferenceType() || !DestType->isRecordType())
+ return;
----------------
The reference type check here is redundant: a record type is never a reference type.
================
Comment at: lib/Sema/SemaInit.cpp:7352-7353
@@ +7351,4 @@
+ dyn_cast<CXXConstructExpr>(InitExpr->IgnoreParens());
+ if (!CCE || !CCE->getConstructor()->isCopyOrMoveConstructor())
+ return;
+
----------------
This check seems to prevent your redundant_move_on_return warning from firing at the right times.
================
Comment at: lib/Sema/SemaInit.cpp:7377-7378
@@ +7376,4 @@
+
+ const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl());
+ if (!VD || !VD->hasLocalStorage())
+ return;
----------------
You should also check that `VD` is from the current function/lambda/whatever and not an enclosing one (using `DRE->refersToEnclosingVariableOrCapture`).
================
Comment at: lib/Sema/SemaInit.cpp:7420-7423
@@ -7361,1 +7419,6 @@
+ if (const Expr *E = Result.get()) {
+ CheckMoveOnConstruction(*this, E,
+ Entity.getKind() == InitializedEntity::EK_Result);
+ }
+
----------------
Can you put this inside `Perform` so that all the users of `InitializationSequence` are guaranteed to reach it? There's lots of places that perform copy elision but don't use this convenience wrapper.
================
Comment at: test/SemaCXX/warn-pessmizing-move.cpp:38-43
@@ +37,8 @@
+B test2(A a1, B b1) {
+ // Object is different than return type so don't warn.
+ A a2;
+ return a1;
+ return a2;
+ return std::move(a1);
+ return std::move(a2);
+
----------------
We should give the "redundant move" warning for this case, but we won't because we didn't use a copy or move constructor.
http://reviews.llvm.org/D7633
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list