[PATCH] D132290: [clang-tidy] Skip unions in use-equals-default

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov created this revision.
alexander-shaposhnikov added reviewers: aaron.ballman, asoffer.
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.

For unions constructors with empty bodies behave differently (in comparison with structs/classes)
(e.g. https://godbolt.org/z/dqd4YGW7j)
and clang-tidy's fix might break the code in some cases : 
https://godbolt.org/z/GaT1bqGGb vs https://godbolt.org/z/4nGTqWvTM
This diff adjusts the check to skip unions for now (it seems to be a relatively rare case).

Test plan: ninja check-clang-tools


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132290

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.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.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,17 @@
   ~NE() { f(); }
 };
 
+// Skip unions.
+union NU {
+  NU() {}
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: NU() {}
+  ~NU() {}
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: use '= default'
+  // 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,20 @@
   int Field;
 };
 
+// Skip unions.
+union NU {
+  NU(const NU &Other) : Field(Other.Field) {}
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: NU(const NU &Other) :
+  NU &operator=(const NU &Other) {
+    Field = Other.Field;
+    return *this;
+  }
+  // CHECK-MESSAGES-NOT: :[[@LINE-4]]:7: warning: use '= default'
+  // CHECK-FIXES: NU &operator=(const NU &Other) {
+  IL Field;
+};
+
 // Wrong type.
 struct WT {
   WT(const IL &Other) {}
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
@@ -218,11 +218,13 @@
 
 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
   // 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 +239,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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132290.454199.patch
Type: text/x-patch
Size: 2919 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220820/dbe9695e/attachment.bin>