Why are these DefaultIgnore? On Tue, Apr 28, 2015 at 6:52 PM, Richard Trieu <[email protected]> wrote:
> Author: rtrieu > Date: Tue Apr 28 20:52:17 2015 > New Revision: 236075 > > URL: http://llvm.org/viewvc/llvm-project?rev=236075&view=rev > Log: > Add -Wpessimizing-move and -Wredundant-move warnings. > > -Wpessimizing-move warns when a call to std::move would prevent copy > elision > if the argument was not wrapped in a call. This happens when moving a > local > variable in a return statement when the variable is the same type as the > return type or using a move to create a new object from a temporary object. > > -Wredundant-move warns when an implicit move would already be made, so the > std::move call is not needed, such as when moving a local variable in a > return > that is different from the return type. > > Differential Revision: http://reviews.llvm.org/D7633 > > Added: > cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp > cfe/trunk/test/SemaCXX/warn-redundant-move.cpp > Modified: > cfe/trunk/include/clang/Basic/DiagnosticGroups.td > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/SemaInit.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=236075&r1=236074&r2=236075&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Apr 28 20:52:17 > 2015 > @@ -286,6 +286,7 @@ def DeprecatedObjCIsaUsage : DiagGroup<" > 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<"pointer-to-int-cast">; > 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]>; > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=236075&r1=236074&r2=236075&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 28 > 20:52:17 2015 > @@ -4777,6 +4777,17 @@ def warn_self_move : Warning< > "explicitly moving variable of type %0 to itself">, > InGroup<SelfMove>, DefaultIgnore; > > +def warn_redundant_move_on_return : Warning< > + "redundant move in return statement">, > + InGroup<RedundantMove>, 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>; > > Modified: cfe/trunk/lib/Sema/SemaInit.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=236075&r1=236074&r2=236075&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaInit.cpp (original) > +++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Apr 28 20:52:17 2015 > @@ -5768,6 +5768,112 @@ static void DiagnoseNarrowingInInitList( > 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<CXXConstructExpr>(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<MaterializeTemporaryExpr>(InitExpr)) { > + InitExpr = MTE->GetTemporaryExpr()->IgnoreImpCasts(); > + while (const CXXConstructExpr *CCE = > + dyn_cast<CXXConstructExpr>(InitExpr)) { > + if (isa<CXXTemporaryObjectExpr>(CCE)) > + return; > + if (CCE->getNumArgs() == 0) > + return; > + if (CCE->getNumArgs() > 1 && > !isa<CXXDefaultArgExpr>(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<CallExpr>(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<DeclRefExpr>(Arg->IgnoreParenImpCasts()); > + if (!DRE || DRE->refersToEnclosingVariableOrCapture()) > + return; > + > + const VarDecl *VD = dyn_cast<VarDecl>(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 @@ InitializationSequence::Perform(Sema &S, > cast<FieldDecl>(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; > } > > > Added: cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp?rev=236075&view=auto > > ============================================================================== > --- cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp (added) > +++ cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp Tue Apr 28 20:52:17 > 2015 > @@ -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 <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}} > + // 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 > +} > > Added: cfe/trunk/test/SemaCXX/warn-redundant-move.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-redundant-move.cpp?rev=236075&view=auto > > ============================================================================== > --- cfe/trunk/test/SemaCXX/warn-redundant-move.cpp (added) > +++ cfe/trunk/test/SemaCXX/warn-redundant-move.cpp Tue Apr 28 20:52:17 2015 > @@ -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 <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 : 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}:"" > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
