[PATCH] D146882: [clang-tidy] Correct union & macros handling in modernize-use-equals-default
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb87e55c9aad: [clang-tidy] Correct union macros
handling in modernize-use-equals-default (authored by PiotrZSL).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146882/new/
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
@@ -203,6 +203,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
+ ` 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
` 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
[PATCH] D146882: [clang-tidy] Correct union & macros handling in modernize-use-equals-default
alexander-shaposhnikov accepted this revision. alexander-shaposhnikov added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146882/new/ https://reviews.llvm.org/D146882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146882: [clang-tidy] Correct union & macros handling in modernize-use-equals-default
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
+ ` 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
` 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(),
