diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h --- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h @@ -34,15 +34,16 @@ /// 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 getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } private: const bool IgnoreMacros; diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp @@ -15,32 +15,53 @@ 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(&Node)) + return Constructor->isDefaultConstructor() || + Constructor->isCopyOrMoveConstructor(); + + return isa(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 @@ 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 @@ // 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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -220,6 +220,10 @@ ` to support for-loops with iterators initialized by free functions like ``begin``, ``end``, or ``size``. +- Improved :doc:`modernize-use-equals-delete + ` check to ignore + false-positives when special member function is actually used or implicit. + - Improved :doc:`performance-faster-string-find ` check to properly escape single quotes. 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 --- 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 @@ 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(); + }; +}