Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td +++ cfe/trunk/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">, @@ -294,6 +295,7 @@ def : DiagGroup<"redundant-decls">; def RedeclaredClassMember : DiagGroup<"redeclared-class-member">; def GNURedeclaredEnum : DiagGroup<"gnu-redeclared-enum">; +def RedundantMove : DiagGroup<"redundant-move">; def ReturnStackAddress : DiagGroup<"return-stack-address">; def ReturnTypeCLinkage : DiagGroup<"return-type-c-linkage">; def ReturnType : DiagGroup<"return-type", [ReturnTypeCLinkage]>; Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -4777,6 +4777,17 @@ "explicitly moving variable of type %0 to itself">, InGroup, DefaultIgnore; +def warn_redundant_move_on_return : Warning< + "redundant move in return statement">, + InGroup, DefaultIgnore; +def warn_pessimizing_move_on_return : Warning< + "moving a local object in a return statement prevents copy elision">, + InGroup, DefaultIgnore; +def warn_pessimizing_move_on_initialization : Warning< + "moving a temporary object prevents copy elision">, + InGroup, 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; Index: cfe/trunk/lib/Sema/SemaInit.cpp =================================================================== --- cfe/trunk/lib/Sema/SemaInit.cpp +++ cfe/trunk/lib/Sema/SemaInit.cpp @@ -5768,6 +5768,112 @@ QualType EntityType, const Expr *PostInit); +/// Provide warnings when std::move is used on construction. +static void CheckMoveOnConstruction(Sema &S, const Expr *InitExpr, + bool IsReturnStmt) { + if (!InitExpr) + return; + + QualType DestType = InitExpr->getType(); + if (!DestType->isRecordType()) + return; + + unsigned DiagID = 0; + if (IsReturnStmt) { + const CXXConstructExpr *CCE = + dyn_cast(InitExpr->IgnoreParens()); + if (!CCE || CCE->getNumArgs() != 1) + return; + + if (!CCE->getConstructor()->isCopyOrMoveConstructor()) + return; + + InitExpr = CCE->getArg(0)->IgnoreImpCasts(); + + // Remove implicit temporary and constructor nodes. + if (const MaterializeTemporaryExpr *MTE = + dyn_cast(InitExpr)) { + InitExpr = MTE->GetTemporaryExpr()->IgnoreImpCasts(); + while (const CXXConstructExpr *CCE = + dyn_cast(InitExpr)) { + if (isa(CCE)) + return; + if (CCE->getNumArgs() == 0) + return; + if (CCE->getNumArgs() > 1 && !isa(CCE->getArg(1))) + return; + InitExpr = CCE->getArg(0); + } + InitExpr = InitExpr->IgnoreImpCasts(); + DiagID = diag::warn_redundant_move_on_return; + } + } + + // Find the std::move call and get the argument. + const CallExpr *CE = dyn_cast(InitExpr->IgnoreParens()); + 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 (IsReturnStmt) { + const DeclRefExpr *DRE = dyn_cast(Arg->IgnoreParenImpCasts()); + if (!DRE || DRE->refersToEnclosingVariableOrCapture()) + return; + + const VarDecl *VD = dyn_cast(DRE->getDecl()); + if (!VD || !VD->hasLocalStorage()) + return; + + if (DiagID == 0) { + DiagID = S.Context.hasSameUnqualifiedType(DestType, VD->getType()) + ? diag::warn_pessimizing_move_on_return + : diag::warn_redundant_move_on_return; + } + } else { + DiagID = diag::warn_pessimizing_move_on_initialization; + const Expr *ArgStripped = Arg->IgnoreImplicit()->IgnoreParens(); + if (!ArgStripped->isRValue() || !ArgStripped->getType()->isRecordType()) + return; + } + + S.Diag(CE->getLocStart(), DiagID); + + // Get all the locations for a fix-it. Don't emit the fix-it if any location + // is within a macro. + SourceLocation CallBegin = CE->getCallee()->getLocStart(); + if (CallBegin.isMacroID()) + return; + SourceLocation RParen = CE->getRParenLoc(); + if (RParen.isMacroID()) + return; + SourceLocation LParen; + SourceLocation ArgLoc = Arg->getLocStart(); + + // Special testing for the argument location. Since the fix-it needs the + // location right before the argument, the argument location can be in a + // macro only if it is at the beginning of the macro. + while (ArgLoc.isMacroID() && + S.getSourceManager().isAtStartOfImmediateMacroExpansion(ArgLoc)) { + ArgLoc = S.getSourceManager().getImmediateExpansionRange(ArgLoc).first; + } + + if (LParen.isMacroID()) + return; + + LParen = ArgLoc.getLocWithOffset(-1); + + S.Diag(CE->getLocStart(), diag::note_remove_move) + << FixItHint::CreateRemoval(SourceRange(CallBegin, LParen)) + << FixItHint::CreateRemoval(SourceRange(RParen, RParen)); +} + ExprResult InitializationSequence::Perform(Sema &S, const InitializedEntity &Entity, @@ -6497,6 +6603,12 @@ cast(Entity.getDecl()), CurInit.get()); + // Check for std::move on construction. + if (const Expr *E = CurInit.get()) { + CheckMoveOnConstruction(S, E, + Entity.getKind() == InitializedEntity::EK_Result); + } + return CurInit; } Index: cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp =================================================================== --- cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp +++ cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp @@ -0,0 +1,203 @@ +// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-move -std=c++11 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// definitions for std::move +namespace std { +inline namespace foo { +template struct remove_reference { typedef T type; }; +template struct remove_reference { typedef T type; }; +template struct remove_reference { typedef T type; }; + +template typename remove_reference::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}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + return std::move(a2); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" +} + +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}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + return std::move(b2); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" +} + +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}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:"" + return std::move(test3()); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:27-[[@LINE-4]]:28}:"" +} + +void test5(A) { + test5(A()); + test5(test4()); + + test5(std::move(A())); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:19}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + test5(std::move(test4())); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:19}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:27}:"" +} + +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}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:"" + A a4 = std::move(test3()); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:27-[[@LINE-4]]:28}:"" +} + +A test7() { + A a1 = std::move(A()); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:"" + A a2 = std::move((A())); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:26}:"" + A a3 = (std::move(A())); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:25}:"" + A a4 = (std::move((A()))); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:27}:"" + + return std::move(a1); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + return std::move((a1)); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:25}:"" + return (std::move(a1)); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:"" + return (std::move((a1))); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:26}:"" +} + +#define wrap1(x) x +#define wrap2(x) x + +// Macro test. Since the std::move call is outside the macro, it is +// safe to suggest a fix-it. +A test8(A a) { + return std::move(a); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:22}:"" + return std::move(wrap1(a)); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:28-[[@LINE-4]]:29}:"" + return std::move(wrap1(wrap2(a))); + // expected-warning@-1{{prevents copy elision}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:35-[[@LINE-4]]:36}:"" +} + +#define test9 \ + A test9(A a) { \ + return std::move(a); \ + } + +// Macro test. The std::call is inside the macro, so no fix-it is suggested. +test9 +// expected-warning@-1{{prevents copy elision}} +// CHECK-NOT: fix-it + +#define return_a return std::move(a) + +// Macro test. The std::call is inside the macro, so no fix-it is suggested. +A test10(A a) { + return_a; + // expected-warning@-1{{prevents copy elision}} + // CHECK-NOT: fix-it +} Index: cfe/trunk/test/SemaCXX/warn-redundant-move.cpp =================================================================== --- cfe/trunk/test/SemaCXX/warn-redundant-move.cpp +++ cfe/trunk/test/SemaCXX/warn-redundant-move.cpp @@ -0,0 +1,68 @@ +// RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// definitions for std::move +namespace std { +inline namespace foo { +template struct remove_reference { typedef T type; }; +template struct remove_reference { typedef T type; }; +template struct remove_reference { typedef T type; }; + +template typename remove_reference::type &&move(T &&t); +} +} + +struct A {}; +struct B : public A {}; + +A test1(B b1) { + B b2; + //return b1; + //return b2; + return std::move(b1); + // expected-warning@-1{{redundant move}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + return std::move(b2); + // expected-warning@-1{{redundant move}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" +} + +struct C { + C() {} + C(A) {} +}; + +C test2(A a1, B b1) { + A a2; + B b2; + + return a1; + return a2; + return b1; + return b2; + + return std::move(a1); + // expected-warning@-1{{redundant move}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + return std::move(a2); + // expected-warning@-1{{redundant move}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + return std::move(b1); + // expected-warning@-1{{redundant move}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" + return std::move(b2); + // expected-warning@-1{{redundant move}} + // expected-note@-2{{remove std::move call}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:"" +}