Author: Logan Gnanapragasam Date: 2023-04-03T17:01:59Z New Revision: e51f46705e2876fbbdb59d4f1ba43fa6ecf78611
URL: https://github.com/llvm/llvm-project/commit/e51f46705e2876fbbdb59d4f1ba43fa6ecf78611 DIFF: https://github.com/llvm/llvm-project/commit/e51f46705e2876fbbdb59d4f1ba43fa6ecf78611.diff LOG: [clang-tidy] ignore NRVO const variables in performance-no-automatic-move. In the following code: ```cc struct Obj { Obj(); Obj(const Obj &); Obj(Obj &&); virtual ~Obj(); }; Obj ConstNrvo() { const Obj obj; return obj; } ``` performance-no-automatic-move warns about the constness of `obj`. However, NRVO is applied to `obj`, so the `const` should have no effect on performance. This change modifies the matcher to exclude NRVO variables. #clang-tidy Reviewed By: courbet, PiotrZSL Differential Revision: https://reviews.llvm.org/D147419 Added: Modified: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp index b9c411e26699..42bf8ff83186 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 8a82e4e5ca56..13a06efcf563 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -275,6 +275,10 @@ Changes in existing checks <clang-tidy/checks/google/readability-avoid-underscore-in-googletest-name>` when using ``DISABLED_`` in the test suite name. +- Fixed a false positive in :doc:`performance-no-automatic-move + <clang-tidy/checks/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 6ca59b6bb902..d365f7de8b7c 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<Obj>(); - 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<Obj> PositiveStatusOrLifetimeExtension() { // Negatives. StatusOr<Obj> Temporary() { - return Make<const Obj>(); + return Make<Obj>(); } StatusOr<Obj> ConstTemporary() { return Make<const Obj>(); } -StatusOr<Obj> Nrvo() { +StatusOr<Obj> ConvertingMoveConstructor() { Obj obj = Make<Obj>(); return obj; } +Obj ConstNrvo() { + const Obj obj = Make<Obj>(); + return obj; +} + +Obj NotNrvo(bool b) { + Obj obj1; + Obj obj2; + if (b) { + return obj1; + } + return obj2; +} + StatusOr<Obj> Ref() { Obj &obj = Make<Obj &>(); return obj; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits