[clang-tools-extra] 083e3a1 - [clang-tidy] Skip unions in use-equals-default

[Alexander Shaposhnikov via cfe-commits]

Author: Alexander Shaposhnikov
Date: 2022-08-23T20:09:47Z
New Revision: 083e3a173d9e9d71733e0445e6f4a93e7f918dfa

URL: https://github.com/llvm/llvm-project/commit/083e3a173d9e9d71733e0445e6f4a93e7f918dfa
DIFF: https://github.com/llvm/llvm-project/commit/083e3a173d9e9d71733e0445e6f4a93e7f918dfa.diff

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

For unions constructors with empty bodies behave differently
(in comparison with structs/classes) and clang-tidy's fix
might break the code. This diff adjusts the check to skip unions
for now (it seems to be a relatively rare case).

Test plan: ninja check-all

Differential revision: https://reviews.llvm.org/D132290

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
index c3e3759af32fd..f321c0b2e693d 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -217,12 +217,17 @@ void UseEqualsDefaultCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 }
 
 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
+  // Skip unions since constructors with empty bodies behave 
diff erently
+  // 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 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
       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.

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9e5e176336468..fbe47316d23ae 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,11 @@ Changes in existing checks
   ``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
 ^^^^^^^^^^^^^^
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
index 429e72497b1b4..78a03f02eed52 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
@@ -32,6 +32,18 @@ struct IL {
   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) {}

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
index 40311a1435dd4..1de4957ceb1b9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -33,6 +33,15 @@ class NE {
   ~NE() { f(); }
 };
 
+// Skip unions.
+union NU {
+  NU() {}
+  // CHECK-FIXES: NU() {}
+  ~NU() {}
+  // CHECK-FIXES: ~NU() {}
+  NE Field;
+};
+
 // Initializer or arguments.
 class IA {
 public: