alexander-shaposhnikov created this revision.
alexander-shaposhnikov added reviewers: gribozavr2, njames93.
alexander-shaposhnikov created this object with visibility "All Users".
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
alexander-shaposhnikov requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Skip unions/union-like classes since constructors with empty bodies behave
differently in comparison with regular structs/classes
and clang-tidy's fix might break the code in some cases:
https://godbolt.org/z/86qd4z55e
This is a follow-up to D132290 <https://reviews.llvm.org/D132290>.
With this patch clang-tidy is able to refactor LLVM/Clang/LLD/LLDB without
build breakages.
Test plan: ninja check-clang-tools
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132713
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
@@ -42,6 +42,17 @@
NE Field;
};
+// Skip structs/classes containing anonymous unions.
+struct SU {
+ SU() {}
+ // CHECK-FIXES: SU() {}
+ ~SU() {}
+ // CHECK-FIXES: ~SU() {}
+ union {
+ 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
@@ -44,6 +44,20 @@
IL Field;
};
+// Skip structs/classes containing anonymous unions.
+struct SU {
+ SU(const SU &Other) : Field(Other.Field) {}
+ // CHECK-FIXES: SU(const SU &Other) :
+ SU &operator=(const SU &Other) {
+ Field = Other.Field;
+ return *this;
+ }
+ // CHECK-FIXES: SU &operator=(const SU &Other) {
+ union {
+ 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
@@ -134,8 +134,8 @@
- 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.
+ The check now skips unions/union-like classes 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,17 +217,21 @@
}
void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
- // Skip unions since constructors with empty bodies behave differently
- // in comparison with structs/classes.
+ // Skip union-like classes since constructors with empty bodies behave
+ // differently in comparison with structs/classes.
+
+ auto IsUnionLikeClass = recordDecl(
+ anyOf(isUnion(),
+ has(fieldDecl(isImplicit(), hasType(cxxRecordDecl(isUnion()))))));
// Destructor.
- Finder->addMatcher(cxxDestructorDecl(unless(hasParent(recordDecl(isUnion()))),
- isDefinition())
- .bind(SpecialFunction),
- this);
+ Finder->addMatcher(
+ cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition())
+ .bind(SpecialFunction),
+ this);
Finder->addMatcher(
cxxConstructorDecl(
- unless(hasParent(recordDecl(isUnion()))), isDefinition(),
+ unless(hasParent(IsUnionLikeClass)), isDefinition(),
anyOf(
// Default constructor.
allOf(unless(hasAnyConstructorInitializer(isWritten())),
@@ -242,7 +246,7 @@
this);
// Copy-assignment operator.
Finder->addMatcher(
- cxxMethodDecl(unless(hasParent(recordDecl(isUnion()))), isDefinition(),
+ cxxMethodDecl(unless(hasParent(IsUnionLikeClass)), isDefinition(),
isCopyAssignmentOperator(),
// isCopyAssignmentOperator() allows the parameter to be
// passed by value, and in this case it cannot be
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
