Author: Piotr Zegar Date: 2023-09-03T17:05:31Z New Revision: 208fa9acc0ffe5a460bcd504229c24a8d3f87180
URL: https://github.com/llvm/llvm-project/commit/208fa9acc0ffe5a460bcd504229c24a8d3f87180 DIFF: https://github.com/llvm/llvm-project/commit/208fa9acc0ffe5a460bcd504229c24a8d3f87180.diff LOG: [clang-tidy] Ignore used special-members in modernize-use-equals-delete Special members marked as used, or with out-of-line definition should not raise an warning now. Fixes: #33759 Reviewed By: carlosgalvezp Differential Revision: https://reviews.llvm.org/D158486 Added: Modified: clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-delete.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp index b82c01ee7708cce..059a0af60d3ee84 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp @@ -15,32 +15,53 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { +namespace { +AST_MATCHER(FunctionDecl, hasAnyDefinition) { + if (Node.hasBody() || Node.isPure() || Node.isDefaulted() || Node.isDeleted()) + return true; + + if (const FunctionDecl *Definition = Node.getDefinition()) + if (Definition->hasBody() || Definition->isPure() || + Definition->isDefaulted() || Definition->isDeleted()) + return true; + + return false; +} + +AST_MATCHER(Decl, isUsed) { return Node.isUsed(); } + +AST_MATCHER(CXXMethodDecl, isSpecialFunction) { + if (const auto *Constructor = dyn_cast<CXXConstructorDecl>(&Node)) + return Constructor->isDefaultConstructor() || + Constructor->isCopyOrMoveConstructor(); + + return isa<CXXDestructorDecl>(Node) || Node.isCopyAssignmentOperator() || + Node.isMoveAssignmentOperator(); +} +} // namespace + static const char SpecialFunction[] = "SpecialFunction"; static const char DeletedNotPublic[] = "DeletedNotPublic"; +UseEqualsDeleteCheck::UseEqualsDeleteCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {} + void UseEqualsDeleteCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreMacros", IgnoreMacros); } void UseEqualsDeleteCheck::registerMatchers(MatchFinder *Finder) { - auto PrivateSpecialFn = cxxMethodDecl( - isPrivate(), - anyOf(cxxConstructorDecl(anyOf(isDefaultConstructor(), - isCopyConstructor(), isMoveConstructor())), - cxxMethodDecl( - anyOf(isCopyAssignmentOperator(), isMoveAssignmentOperator())), - cxxDestructorDecl())); + auto PrivateSpecialFn = cxxMethodDecl(isPrivate(), isSpecialFunction()); Finder->addMatcher( cxxMethodDecl( - PrivateSpecialFn, - unless(anyOf(hasAnyBody(stmt()), isDefaulted(), isDeleted(), - ast_matchers::isTemplateInstantiation(), - // Ensure that all methods except private special member - // functions are defined. - hasParent(cxxRecordDecl(hasMethod(unless( - anyOf(PrivateSpecialFn, hasAnyBody(stmt()), isPure(), - isDefaulted(), isDeleted())))))))) + PrivateSpecialFn, unless(hasAnyDefinition()), unless(isUsed()), + // Ensure that all methods except private special member functions are + // defined. + unless(ofClass(hasMethod(cxxMethodDecl(unless(PrivateSpecialFn), + unless(hasAnyDefinition())))))) .bind(SpecialFunction), this); @@ -55,7 +76,7 @@ void UseEqualsDeleteCheck::check(const MatchFinder::MatchResult &Result) { SourceLocation EndLoc = Lexer::getLocForEndOfToken( Func->getEndLoc(), 0, *Result.SourceManager, getLangOpts()); - if (Func->getLocation().isMacroID() && IgnoreMacros) + if (IgnoreMacros && Func->getLocation().isMacroID()) return; // FIXME: Improve FixItHint to make the method public. diag(Func->getLocation(), @@ -66,7 +87,7 @@ void UseEqualsDeleteCheck::check(const MatchFinder::MatchResult &Result) { // Ignore this warning in macros, since it's extremely noisy in code using // DISALLOW_COPY_AND_ASSIGN-style macros and there's no easy way to // automatically fix the warning when macros are in play. - if (Func->getLocation().isMacroID() && IgnoreMacros) + if (IgnoreMacros && Func->getLocation().isMacroID()) return; // FIXME: Add FixItHint to make the method public. diag(Func->getLocation(), "deleted member function should be public"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h index 7870e18e09279c7..a04ffd53320d058 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h @@ -34,15 +34,16 @@ namespace clang::tidy::modernize { /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-equals-delete.html class UseEqualsDeleteCheck : public ClangTidyCheck { public: - UseEqualsDeleteCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {} + UseEqualsDeleteCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } private: const bool IgnoreMacros; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 740c1c87e054ecb..5dfda9928aca20b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -234,6 +234,10 @@ Changes in existing checks <clang-tidy/checks/modernize/loop-convert>` to support for-loops with iterators initialized by free functions like ``begin``, ``end``, or ``size``. +- Improved :doc:`modernize-use-equals-delete + <clang-tidy/checks/modernize/use-equals-delete>` check to ignore + false-positives when special member function is actually used or implicit. + - Improved :doc:`modernize-use-std-print <clang-tidy/checks/modernize/use-std-print>` check to accurately generate fixes for reordering arguments. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-delete.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-delete.cpp index 988c8e5b2b914bf..061a2722b1ac758 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-delete.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-delete.cpp @@ -191,3 +191,31 @@ class C { private: MACRO(C); }; + +namespace PR33759 { + + class Number { + private: + Number(); + ~Number(); + + public: + static Number& getNumber() { + static Number number; + return number; + } + + int getIntValue() { return (int)someFloat; } + float getFloatValue() { return someFloat; } + private: + float someFloat; + }; + + class Number2 { + private: + Number2(); + ~Number2(); + public: + static Number& getNumber(); + }; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
