[clang-tools-extra] 208fa9a - [clang-tidy] Ignore used special-members in modernize-use-equals-delete
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 3 10:48:13 PDT 2023
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();
+ };
+}
More information about the cfe-commits
mailing list