[PATCH] D38179: [clang-tidy] Handle unions in modernize-use-equals-default check

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons created this revision.
Herald added subscribers: xazax.hun, JDevlieghere.

Do not replace the constructor or destructor of a union with =default
if it would be implicitly deleted.

Fixes: PR27926


https://reviews.llvm.org/D38179

Files:
  clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  test/clang-tidy/modernize-use-equals-default.cpp


Index: test/clang-tidy/modernize-use-equals-default.cpp
===================================================================
--- test/clang-tidy/modernize-use-equals-default.cpp
+++ test/clang-tidy/modernize-use-equals-default.cpp
@@ -205,3 +205,23 @@
   };
 
 STRUCT_WITH_DEFAULT(unsigned char, InMacro)
+
+// Unions
+union UnionOfPOD {
+  UnionOfPOD() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: UnionOfPOD() = default;
+  ~UnionOfPOD() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~UnionOfPOD() = default;
+  int i;
+  double j;
+};
+
+union UnionOfNonTrivial {
+  UnionOfNonTrivial() {}
+  ~UnionOfNonTrivial() {}
+  int i;
+  double j;
+  NE k;
+};
Index: clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===================================================================
--- clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -111,6 +111,44 @@
          BasesToInit.size() + FieldsToInit.size();
 }
 
+/// Checks that the given default constructor can be defaulted.
+static bool defaultConstructorCanBeDefaulted(ASTContext *Context,
+                                             const CXXMethodDecl *Operator) {
+  const auto *Record = Operator->getParent();
+
+  // A defaulted default constructor of a union with a field with a non trivial
+  // default constructor would be deleted.
+  if (Record->isUnion()) {
+    auto FieldsToInit = getAllNamedFields(Record);
+
+    for (const auto *Field : FieldsToInit) {
+      QualType T = Context->getBaseElementType(Field->getType());
+
+      if (const RecordType *RecordTy = T->getAs<RecordType>()) {
+        CXXRecordDecl *FieldRec = cast<CXXRecordDecl>(RecordTy->getDecl());
+
+        if (FieldRec->hasNonTrivialDefaultConstructor())
+          return false;
+      }
+    }
+  }
+
+  return true;
+}
+
+/// Checks that the given destructor can be defaulted.
+static bool destructorCanBeDefaulted(ASTContext *Context,
+                                     const CXXMethodDecl *Operator) {
+  const auto *Record = Operator->getParent();
+
+  // A defaulted destructor of a union with a field with a non trivial
+  // destructor would be deleted.
+  if (Record->defaultedDestructorIsDeleted())
+    return false;
+
+  return true;
+}
+
 /// \brief Checks that the given method is an overloading of the assignment
 /// operator, has copy signature, returns a reference to "*this" and copies
 /// all its members and subobjects.
@@ -274,6 +312,8 @@
 
   if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(SpecialFunctionDecl)) {
     if (Ctor->getNumParams() == 0) {
+      if (!defaultConstructorCanBeDefaulted(Result.Context, Ctor))
+        return;
       SpecialFunctionName = "default constructor";
     } else {
       if (!isCopyConstructorAndCanBeDefaulted(Result.Context, Ctor))
@@ -286,6 +326,8 @@
       }
     }
   } else if (isa<CXXDestructorDecl>(SpecialFunctionDecl)) {
+    if (!destructorCanBeDefaulted(Result.Context, SpecialFunctionDecl))
+      return;
     SpecialFunctionName = "destructor";
   } else {
     if (!isCopyAssignmentAndCanBeDefaulted(Result.Context, SpecialFunctionDecl))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38179.116351.patch
Type: text/x-patch
Size: 3192 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170922/865c3199/attachment.bin>