diff --git a/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp index b9c411e266998..42bf8ff831868 100644 --- a/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp @@ -16,6 +16,12 @@ using namespace clang::ast_matchers; namespace clang::tidy::performance { +namespace { + +AST_MATCHER(VarDecl, isNRVOVariable) { return Node.isNRVOVariable(); } + +} // namespace + NoAutomaticMoveCheck::NoAutomaticMoveCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -23,8 +29,9 @@ NoAutomaticMoveCheck::NoAutomaticMoveCheck(StringRef Name, utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) { - const auto ConstLocalVariable = + const auto NonNrvoConstLocalVariable = varDecl(hasLocalStorage(), unless(hasType(lValueReferenceType())), + unless(isNRVOVariable()), hasType(qualType( isConstQualified(), hasCanonicalType(matchers::isExpensiveToCopy()), @@ -48,7 +55,7 @@ void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) { cxxConstructExpr( hasDeclaration(LValueRefCtor), hasArgument(0, ignoringParenImpCasts(declRefExpr( - to(ConstLocalVariable))))) + to(NonNrvoConstLocalVariable))))) .bind("ctor_call")))))), this); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8a82e4e5ca56a..13a06efcf563d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -275,6 +275,10 @@ Changes in existing checks ` when using ``DISABLED_`` in the test suite name. +- Fixed a false positive in :doc:`performance-no-automatic-move + ` when warning would be + emitted for a const local variable to which NRVO is applied. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp index 6ca59b6bb902c..d365f7de8b7c1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp @@ -33,10 +33,15 @@ NonTemplate PositiveNonTemplateConstValue() { // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move] } -Obj PositiveSelfConstValue() { - const Obj obj = Make(); - return obj; - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move] +Obj PositiveCantNrvo(bool b) { + const Obj obj1; + const Obj obj2; + if (b) { + return obj1; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents automatic move [performance-no-automatic-move] + } + return obj2; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj2' prevents automatic move [performance-no-automatic-move] } // FIXME: Ideally we would warn here too. @@ -54,18 +59,32 @@ StatusOr PositiveStatusOrLifetimeExtension() { // Negatives. StatusOr Temporary() { - return Make(); + return Make(); } StatusOr ConstTemporary() { return Make(); } -StatusOr Nrvo() { +StatusOr ConvertingMoveConstructor() { Obj obj = Make(); return obj; } +Obj ConstNrvo() { + const Obj obj = Make(); + return obj; +} + +Obj NotNrvo(bool b) { + Obj obj1; + Obj obj2; + if (b) { + return obj1; + } + return obj2; +} + StatusOr Ref() { Obj &obj = Make(); return obj;