[clang-tidy] Ignore used special-members in modernize-use-equals-delete

[PiotrZSL]

Summary

Special members marked as used, or with out-of-line
definition should not raise an warning now.

Fixes: #33759

[carlosgalvezp]

LGTM, thanks for fixing!

clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h

 ///     A& operator=(const A&) = delete;
 ///   };
 /// \endcode
 ///
 /// For the user-facing documentation see:
 /// 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;
 };
 
 } // namespace clang::tidy::modernize
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EQUALS_DELETE_H

clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.cpp

 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 
 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);
 
   Finder->addMatcher(
       cxxMethodDecl(isDeleted(), unless(isPublic())).bind(DeletedNotPublic),
       this);
 }
 
 void UseEqualsDeleteCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Func =
           Result.Nodes.getNodeAs<CXXMethodDecl>(SpecialFunction)) {
     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(),
          "use '= delete' to prohibit calling of a special member function")
         << FixItHint::CreateInsertion(EndLoc, " = delete");
   } else if (const auto *Func =
                  Result.Nodes.getNodeAs<CXXMethodDecl>(DeletedNotPublic)) {
     // 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");
   }
 }
 
 } // namespace clang::tidy::modernize

clang-tools-extra/docs/ReleaseNotes.rst

 - Improved :doc:`misc-redundant-expression
   <clang-tidy/checks/misc/redundant-expression>` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``).
 
 - Improved :doc:`modernize-loop-convert
   <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:`performance-faster-string-find
   <clang-tidy/checks/performance/faster-string-find>` check to properly escape
   single quotes.
 
 - Improved :doc:`performance-noexcept-swap
   <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter
   match with the swap function signature, eliminating false-positives.
 

clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-delete.cpp

 };
 
 // This resulted in a warning by default.
 #define MACRO(type) void operator=(type const &)
 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();
+  };
+}