[clang-tools-extra] eb87e55 - [clang-tidy] Correct union & macros handling in modernize-use-equals-default
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 26 06:24:06 PDT 2023
Author: Piotr Zegar
Date: 2023-03-26T13:22:44Z
New Revision: eb87e55c9aade5d7b42606487e69b5f3a6da1e65
URL: https://github.com/llvm/llvm-project/commit/eb87e55c9aade5d7b42606487e69b5f3a6da1e65
DIFF: https://github.com/llvm/llvm-project/commit/eb87e55c9aade5d7b42606487e69b5f3a6da1e65.diff
LOG: [clang-tidy] Correct union & macros handling in modernize-use-equals-default
To this moment this check were ignoring only inline
union special members, From now also out-of-line
special members going to be ignored. Also extended
support for IgnoreMacros to cover also macros used
inside a body, or used preprocesor directives.
Fixes:
- https://github.com/llvm/llvm-project/issues/28300
- https://github.com/llvm/llvm-project/issues/40554
Reviewed By: alexander-shaposhnikov
Differential Revision: https://reviews.llvm.org/D146882
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
index d48d43f2677f0..afac9f7497ac8 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -235,19 +235,19 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
// Destructor.
Finder->addMatcher(
- cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition())
+ cxxDestructorDecl(isDefinition(), unless(ofClass(IsUnionLikeClass)))
.bind(SpecialFunction),
this);
+ // Constructor.
Finder->addMatcher(
cxxConstructorDecl(
- unless(
- hasParent(decl(anyOf(IsUnionLikeClass, functionTemplateDecl())))),
- isDefinition(),
+ isDefinition(), unless(ofClass(IsUnionLikeClass)),
+ unless(hasParent(functionTemplateDecl())),
anyOf(
// Default constructor.
- allOf(unless(hasAnyConstructorInitializer(isWritten())),
- unless(isVariadic()), parameterCountIs(0),
- IsPublicOrOutOfLineUntilCPP20),
+ allOf(parameterCountIs(0),
+ unless(hasAnyConstructorInitializer(isWritten())),
+ unless(isVariadic()), IsPublicOrOutOfLineUntilCPP20),
// Copy constructor.
allOf(isCopyConstructor(),
// Discard constructors that can be used as a copy
@@ -258,9 +258,9 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
this);
// Copy-assignment operator.
Finder->addMatcher(
- cxxMethodDecl(unless(hasParent(
- decl(anyOf(IsUnionLikeClass, functionTemplateDecl())))),
- isDefinition(), isCopyAssignmentOperator(),
+ cxxMethodDecl(isDefinition(), isCopyAssignmentOperator(),
+ unless(ofClass(IsUnionLikeClass)),
+ unless(hasParent(functionTemplateDecl())),
// isCopyAssignmentOperator() allows the parameter to be
// passed by value, and in this case it cannot be
// defaulted.
@@ -299,6 +299,12 @@ void UseEqualsDefaultCheck::check(const MatchFinder::MatchResult &Result) {
if (!SpecialFunctionDecl->isCopyAssignmentOperator() && !Body->body_empty())
return;
+ // If body contain any preprocesor derictives, don't warn.
+ if (IgnoreMacros && utils::lexer::rangeContainsExpansionsOrDirectives(
+ Body->getSourceRange(), *Result.SourceManager,
+ Result.Context->getLangOpts()))
+ return;
+
// If there are comments inside the body, don't do the change.
bool ApplyFix = SpecialFunctionDecl->isCopyAssignmentOperator() ||
bodyEmpty(Result.Context, Body);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 38a64616410ac..1a53aac8fecf4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -203,6 +203,11 @@ Changes in existing checks
constructors toward hand written constructors so that they are skipped if more
than one exists.
+- Fixed false positive in :doc:`modernize-use-equals-default
+ <clang-tidy/checks/modernize/use-equals-default>` check for special member
+ functions containing macros or preprocessor directives, and out-of-line special
+ member functions in unions.
+
- Fixed reading `HungarianNotation.CString.*` options in
:doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
index 5509a5414f8c5..7bd6fa93ba1ea 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
@@ -32,5 +32,6 @@ Options
.. option:: IgnoreMacros
- If set to `true`, the check will not give warnings inside macros. Default
- is `true`.
+ If set to `true`, the check will not give warnings inside macros and will
+ ignore special members with bodies contain macros or preprocessor directives.
+ Default is `true`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
index cc5d379b3c3de..a1d6c25e6364a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -42,6 +42,18 @@ union NU {
NE Field;
};
+// Skip unions with out-of-line constructor/destructor.
+union NUO {
+ NUO();
+ ~NUO();
+ NE Field;
+};
+
+NUO::NUO() {}
+// CHECK-FIXES: NUO::NUO() {}
+NUO::~NUO() {}
+// CHECK-FIXES: NUO::~NUO() {}
+
// Skip structs/classes containing anonymous unions.
struct SU {
SU() {}
@@ -266,3 +278,21 @@ OTC::~OTC() try {} catch(...) {}
};
STRUCT_WITH_DEFAULT(unsigned char, InMacro)
+
+#define ZERO_VALUE 0
+struct PreprocesorDependentTest
+{
+ void something();
+
+ PreprocesorDependentTest() {
+#if ZERO_VALUE
+ something();
+#endif
+ }
+
+ ~PreprocesorDependentTest() {
+#if ZERO_VALUE
+ something();
+#endif
+ }
+};
More information about the cfe-commits
mailing list