Author: rtrieu Date: Wed Aug 7 17:12:51 2019 New Revision: 368237 URL: http://llvm.org/viewvc/llvm-project?rev=368237&view=rev Log: Update fix-it hints for std::move warnings.
Fix -Wpessimizing-move and -Wredundant-move when warning on initializer lists. The new fix-it hints for removing the std::move call will now also suggest removing the braces for the initializer list so that the resulting code will still be compilable. This fixes PR42832 Modified: cfe/trunk/lib/Sema/SemaInit.cpp cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp cfe/trunk/test/SemaCXX/warn-redundant-move.cpp Modified: cfe/trunk/lib/Sema/SemaInit.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368237&r1=368236&r2=368237&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaInit.cpp (original) +++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Aug 7 17:12:51 2019 @@ -7305,27 +7305,34 @@ static void CheckMoveOnConstruction(Sema if (!DestType->isRecordType()) return; - unsigned DiagID = 0; - if (IsReturnStmt) { - const CXXConstructExpr *CCE = - dyn_cast<CXXConstructExpr>(InitExpr->IgnoreParens()); - if (!CCE || CCE->getNumArgs() != 1) - return; + const CXXConstructExpr *CCE = + dyn_cast<CXXConstructExpr>(InitExpr->IgnoreParens()); + if (!CCE || CCE->getNumArgs() != 1) + return; - if (!CCE->getConstructor()->isCopyOrMoveConstructor()) - return; + if (!CCE->getConstructor()->isCopyOrMoveConstructor()) + return; - InitExpr = CCE->getArg(0)->IgnoreImpCasts(); - } + InitExpr = CCE->getArg(0)->IgnoreImpCasts(); // Find the std::move call and get the argument. const CallExpr *CE = dyn_cast<CallExpr>(InitExpr->IgnoreParens()); if (!CE || !CE->isCallToStdMove()) return; - const Expr *Arg = CE->getArg(0)->IgnoreImplicit(); + const Expr *Arg = CE->getArg(0); + + unsigned DiagID = 0; + + if (!IsReturnStmt && !isa<MaterializeTemporaryExpr>(Arg)) + return; - if (IsReturnStmt) { + if (isa<MaterializeTemporaryExpr>(Arg)) { + DiagID = diag::warn_pessimizing_move_on_initialization; + const Expr *ArgStripped = Arg->IgnoreImplicit()->IgnoreParens(); + if (!ArgStripped->isRValue() || !ArgStripped->getType()->isRecordType()) + return; + } else { // IsReturnStmt const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Arg->IgnoreParenImpCasts()); if (!DRE || DRE->refersToEnclosingVariableOrCapture()) return; @@ -7352,24 +7359,18 @@ static void CheckMoveOnConstruction(Sema DiagID = diag::warn_redundant_move_on_return; else DiagID = diag::warn_pessimizing_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->getBeginLoc(), 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()->getBeginLoc(); - if (CallBegin.isMacroID()) + SourceLocation BeginLoc = CCE->getBeginLoc(); + if (BeginLoc.isMacroID()) return; SourceLocation RParen = CE->getRParenLoc(); if (RParen.isMacroID()) return; - SourceLocation LParen; SourceLocation ArgLoc = Arg->getBeginLoc(); // Special testing for the argument location. Since the fix-it needs the @@ -7380,14 +7381,16 @@ static void CheckMoveOnConstruction(Sema ArgLoc = S.getSourceManager().getImmediateExpansionRange(ArgLoc).getBegin(); } + SourceLocation LParen = ArgLoc.getLocWithOffset(-1); if (LParen.isMacroID()) return; - - LParen = ArgLoc.getLocWithOffset(-1); + SourceLocation EndLoc = CCE->getEndLoc(); + if (EndLoc.isMacroID()) + return; S.Diag(CE->getBeginLoc(), diag::note_remove_move) - << FixItHint::CreateRemoval(SourceRange(CallBegin, LParen)) - << FixItHint::CreateRemoval(SourceRange(RParen, RParen)); + << FixItHint::CreateRemoval(SourceRange(BeginLoc, LParen)) + << FixItHint::CreateRemoval(SourceRange(RParen, EndLoc)); } static void CheckForNullPointerDereference(Sema &S, const Expr *E) { Modified: cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp?rev=368237&r1=368236&r2=368237&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp Wed Aug 7 17:12:51 2019 @@ -122,13 +122,13 @@ A test7() { 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}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:26}:"" 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}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:28}:"" return std::move(a1); // expected-warning@-1{{prevents copy elision}} @@ -143,13 +143,13 @@ A test7() { 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}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@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]]:25-[[@LINE-4]]:26}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:27}:"" } #define wrap1(x) x @@ -227,3 +227,30 @@ namespace templates { test2<B, A>(); } } + +A init_list() { + A a1; + 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]]:22}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@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]]:23}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:29}:"" + + 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]]:22}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:28}:"" + + 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]]:23}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:26-[[@LINE-4]]:30}:"" +} Modified: cfe/trunk/test/SemaCXX/warn-redundant-move.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-redundant-move.cpp?rev=368237&r1=368236&r2=368237&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/warn-redundant-move.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-redundant-move.cpp Wed Aug 7 17:12:51 2019 @@ -114,3 +114,17 @@ namespace templates { test2<B, A>(A()); } } + +A init_list(A a) { + return { std::move(a) }; + // expected-warning@-1{{redundant move in return statement}} + // expected-note@-2{{remove std::move call here}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:22}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:26}:"" + + return { (std::move(a)) }; + // expected-warning@-1{{redundant move in return statement}} + // expected-note@-2{{remove std::move call here}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:23}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:28}:"" +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits