PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146882
Files:
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
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -42,6 +42,18 @@
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 @@
};
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
+ }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/use-equals-default.rst
@@ -32,5 +32,6 @@
.. 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`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -196,6 +196,11 @@
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.
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -235,19 +235,19 @@
// 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 @@
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 @@
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
