alexander-shaposhnikov updated this revision to Diff 454470.
alexander-shaposhnikov added a comment.
Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132290/new/
https://reviews.llvm.org/D132290
Files:
clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
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
@@ -33,6 +33,15 @@
~NE() { f(); }
};
+// Skip unions.
+union NU {
+ NU() {}
+ // CHECK-FIXES: NU() {}
+ ~NU() {}
+ // CHECK-FIXES: ~NU() {}
+ NE Field;
+};
+
// Initializer or arguments.
class IA {
public:
Index:
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
@@ -32,6 +32,18 @@
int Field;
};
+// Skip unions.
+union NU {
+ NU(const NU &Other) : Field(Other.Field) {}
+ // CHECK-FIXES: NU(const NU &Other) :
+ NU &operator=(const NU &Other) {
+ Field = Other.Field;
+ return *this;
+ }
+ // CHECK-FIXES: NU &operator=(const NU &Other) {
+ IL Field;
+};
+
// Wrong type.
struct WT {
WT(const IL &Other) {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,11 @@
``push_front`` on STL-style containers and replacing them with ``emplace``
or ``emplace_front``.
+- Improved `modernize-use-equals-default
<clang-tidy/checks/modernize/use-equals-default.html>`_ check.
+
+ The check now skips unions since in this case a default constructor with
empty body
+ is not equivalent to the explicitly defaulted one.
+
Removed checks
^^^^^^^^^^^^^^
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
@@ -217,12 +217,17 @@
}
void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
+ // Skip unions since constructors with empty bodies behave differently
+ // in comparison with structs/classes.
+
// Destructor.
- Finder->addMatcher(cxxDestructorDecl(isDefinition()).bind(SpecialFunction),
+
Finder->addMatcher(cxxDestructorDecl(unless(hasParent(recordDecl(isUnion()))),
+ isDefinition())
+ .bind(SpecialFunction),
this);
Finder->addMatcher(
cxxConstructorDecl(
- isDefinition(),
+ unless(hasParent(recordDecl(isUnion()))), isDefinition(),
anyOf(
// Default constructor.
allOf(unless(hasAnyConstructorInitializer(isWritten())),
@@ -237,7 +242,8 @@
this);
// Copy-assignment operator.
Finder->addMatcher(
- cxxMethodDecl(isDefinition(), isCopyAssignmentOperator(),
+ cxxMethodDecl(unless(hasParent(recordDecl(isUnion()))), isDefinition(),
+ isCopyAssignmentOperator(),
// isCopyAssignmentOperator() allows the parameter to be
// passed by value, and in this case it cannot be
// defaulted.
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
@@ -33,6 +33,15 @@
~NE() { f(); }
};
+// Skip unions.
+union NU {
+ NU() {}
+ // CHECK-FIXES: NU() {}
+ ~NU() {}
+ // CHECK-FIXES: ~NU() {}
+ NE Field;
+};
+
// Initializer or arguments.
class IA {
public:
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
@@ -32,6 +32,18 @@
int Field;
};
+// Skip unions.
+union NU {
+ NU(const NU &Other) : Field(Other.Field) {}
+ // CHECK-FIXES: NU(const NU &Other) :
+ NU &operator=(const NU &Other) {
+ Field = Other.Field;
+ return *this;
+ }
+ // CHECK-FIXES: NU &operator=(const NU &Other) {
+ IL Field;
+};
+
// Wrong type.
struct WT {
WT(const IL &Other) {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,11 @@
``push_front`` on STL-style containers and replacing them with ``emplace``
or ``emplace_front``.
+- Improved `modernize-use-equals-default <clang-tidy/checks/modernize/use-equals-default.html>`_ check.
+
+ The check now skips unions since in this case a default constructor with empty body
+ is not equivalent to the explicitly defaulted one.
+
Removed checks
^^^^^^^^^^^^^^
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
@@ -217,12 +217,17 @@
}
void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
+ // Skip unions since constructors with empty bodies behave differently
+ // in comparison with structs/classes.
+
// Destructor.
- Finder->addMatcher(cxxDestructorDecl(isDefinition()).bind(SpecialFunction),
+ Finder->addMatcher(cxxDestructorDecl(unless(hasParent(recordDecl(isUnion()))),
+ isDefinition())
+ .bind(SpecialFunction),
this);
Finder->addMatcher(
cxxConstructorDecl(
- isDefinition(),
+ unless(hasParent(recordDecl(isUnion()))), isDefinition(),
anyOf(
// Default constructor.
allOf(unless(hasAnyConstructorInitializer(isWritten())),
@@ -237,7 +242,8 @@
this);
// Copy-assignment operator.
Finder->addMatcher(
- cxxMethodDecl(isDefinition(), isCopyAssignmentOperator(),
+ cxxMethodDecl(unless(hasParent(recordDecl(isUnion()))), isDefinition(),
+ isCopyAssignmentOperator(),
// isCopyAssignmentOperator() allows the parameter to be
// passed by value, and in this case it cannot be
// defaulted.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
