http://reviews.llvm.org/D7633
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaInit.cpp
lib/Sema/SemaStmt.cpp
test/SemaCXX/warn-pessmizing-move.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -286,6 +286,7 @@
def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
def Packed : DiagGroup<"packed">;
def Padded : DiagGroup<"padded">;
+def PessimizingMove : DiagGroup<"pessimizing-move">;
def PointerArith : DiagGroup<"pointer-arith">;
def PoundWarning : DiagGroup<"#warnings">;
def PoundPragmaMessage : DiagGroup<"#pragma-messages">,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4764,6 +4764,14 @@
"explicitly moving variable of type %0 to itself">,
InGroup<SelfMove>, DefaultIgnore;
+def warn_pessimizing_move_on_return : Warning<
+ "moving a local object in a return statement prevents copy elision">,
+ InGroup<PessimizingMove>, DefaultIgnore;
+def warn_pessimizing_move_on_initialization : Warning<
+ "moving a temporary object prevents copy elision">,
+ InGroup<PessimizingMove>, DefaultIgnore;
+def note_remove_move : Note<"remove std::move call here">;
+
def warn_string_plus_int : Warning<
"adding %0 to a string does not append to the string">,
InGroup<StringPlusInt>;
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -3239,6 +3239,36 @@
return CandidateSet.BestViableFunction(S, DeclLoc, Best);
}
+static void CheckMoveOnConstruction(Sema &SemaRef, const Expr *InitExpr) {
+ if (!InitExpr)
+ return;
+ if (SemaRef.Diags.isIgnored(diag::warn_pessimizing_move_on_initialization,
+ InitExpr->getLocStart()))
+ return;
+
+ const CallExpr *CE = dyn_cast<CallExpr>(InitExpr);
+ if (!CE || CE->getNumArgs() != 1)
+ return;
+
+ const FunctionDecl *MoveFunction = CE->getDirectCallee();
+ if (!MoveFunction || !MoveFunction->isInStdNamespace() ||
+ !MoveFunction->getIdentifier() ||
+ !MoveFunction->getIdentifier()->isStr("move"))
+ return;
+
+ const Expr *Arg = CE->getArg(0)->IgnoreImplicit();
+ if (!Arg->isRValue() || !Arg->getType()->isRecordType())
+ return;
+
+ SemaRef.Diag(Arg->getLocStart(),
+ diag::warn_pessimizing_move_on_initialization);
+ SemaRef.Diag(InitExpr->getLocStart(), diag::note_remove_move)
+ << FixItHint::CreateRemoval(SourceRange(
+ CE->getLocStart(), Arg->getLocStart().getLocWithOffset(-1)))
+ << FixItHint::CreateRemoval(
+ SourceRange(CE->getRParenLoc(), CE->getRParenLoc()));
+}
+
/// \brief Attempt initialization by constructor (C++ [dcl.init]), which
/// enumerates the constructors of the initialized entity and performs overload
/// resolution to select the best.
@@ -3358,6 +3388,9 @@
Sequence.AddConstructorInitializationStep(
CtorDecl, Best->FoundDecl.getAccess(), DestType, HadMultipleCandidates,
IsListInit | IsInitListCopy, AsInitializerList);
+
+ if (Args.size() == 1)
+ CheckMoveOnConstruction(S, Args[0]);
}
static bool
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -3046,6 +3046,48 @@
return false;
}
+static void CheckMoveOnReturn(Sema &SemaRef, const Expr *ReturnExpr,
+ const FunctionDecl *FD) {
+ if (!ReturnExpr || !FD)
+ return;
+
+ if (SemaRef.Diags.isIgnored(diag::warn_pessimizing_move_on_return,
+ ReturnExpr->getLocStart()))
+ return;
+
+ QualType ReturnType = FD->getReturnType();
+ if (ReturnType->isReferenceType() || !ReturnType->isRecordType())
+ return;
+
+ const CallExpr *CE = dyn_cast<CallExpr>(ReturnExpr);
+ if (!CE || CE->getNumArgs() != 1)
+ return;
+
+ const FunctionDecl *MoveFunction = CE->getDirectCallee();
+ if (!MoveFunction || !MoveFunction->isInStdNamespace() ||
+ !MoveFunction->getIdentifier() ||
+ !MoveFunction->getIdentifier()->isStr("move"))
+ return;
+
+ const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(CE->getArg(0));
+ if (!DRE)
+ return;
+
+ const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl());
+ if (!VD || !VD->hasLocalStorage())
+ return;
+
+ if (!SemaRef.Context.hasSameUnqualifiedType(ReturnType, VD->getType()))
+ return;
+
+ SemaRef.Diag(DRE->getLocStart(), diag::warn_pessimizing_move_on_return);
+ SemaRef.Diag(ReturnExpr->getLocStart(), diag::note_remove_move)
+ << FixItHint::CreateRemoval(SourceRange(
+ CE->getLocStart(), DRE->getLocStart().getLocWithOffset(-1)))
+ << FixItHint::CreateRemoval(
+ SourceRange(CE->getRParenLoc(), CE->getRParenLoc()));
+}
+
StmtResult
Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
Scope *CurScope) {
@@ -3063,6 +3105,8 @@
CheckJumpOutOfSEHFinally(*this, ReturnLoc, *CurScope->getFnParent());
+ CheckMoveOnReturn(*this, RetValExp, getCurFunctionDecl());
+
return R;
}
Index: test/SemaCXX/warn-pessmizing-move.cpp
===================================================================
--- test/SemaCXX/warn-pessmizing-move.cpp
+++ test/SemaCXX/warn-pessmizing-move.cpp
@@ -0,0 +1,96 @@
+// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-move -std=c++11 -verify %s
+
+// definitions for std::move
+namespace std {
+inline namespace foo {
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T&> { typedef T type; };
+template <class T> struct remove_reference<T&&> { typedef T type; };
+
+template <class T> typename remove_reference<T>::type &&move(T &&t);
+}
+}
+
+struct A {};
+struct B {
+ B() {}
+ B(A) {}
+};
+
+A test1(A a1) {
+ A a2;
+ return a1;
+ return a2;
+ return std::move(a1);
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+ return std::move(a2);
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+}
+
+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);
+
+ B b2;
+ return b1;
+ return b2;
+ return std::move(b1);
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+ return std::move(b2);
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+}
+
+A global_a;
+A test3() {
+ // Don't warn when object is not local.
+ return global_a;
+ return std::move(global_a);
+ static A static_a;
+ return static_a;
+ return std::move(static_a);
+
+}
+
+A test4() {
+ return A();
+ return test3();
+
+ return std::move(A());
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+ return std::move(test3());
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+}
+
+void test5(A) {
+ test5(A());
+ test5(test4());
+
+ test5(std::move(A()));
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+ test5(std::move(test4()));
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+}
+
+void test6() {
+ A a1 = A();
+ A a2 = test3();
+
+ A a3 = std::move(A());
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+ A a4 = std::move(test3());
+ // expected-warning@-1{{prevents copy elision}}
+ // expected-note@-2{{remove std::move call}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
