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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.


================
Comment at: clang-tidy/modernize/UseEqualsDefaultCheck.cpp:119-120
+
+  // A defaulted default constructor of a union with a field with a non trivial
+  // default constructor would be deleted.
+  if (Record->isUnion()) {
----------------
malcolm.parsons wrote:
> aaron.ballman wrote:
> > This does not match what's in [class.ctor]p5.
> > 
> > "X is a union that has a variant member with a non-trivial default constructor and no variant member of X has a default member initializer" -- you aren't checking for the default member initializer.
> > 
> > also missing for unions: "X is a union and all of its variant members are of const-qualified type (or array thereof)"
> > 
> > (You should add test cases for these.)
> > 
> > It might make more sense to implement this as `CXXRecordDecl::defaultedDestructorIsDeleted()`?
> Is there any way to call `Sema::ShouldDeleteSpecialMember()` from a clang-tidy check?
By the time clang-tidy checks are invoked, Sema is already dead, and I don't think there's a reasonable way to use it.


================
Comment at: clang-tidy/modernize/UseEqualsDefaultCheck.cpp:127
+
+      if (const RecordType *RecordTy = T->getAs<RecordType>()) {
+        CXXRecordDecl *FieldRec = cast<CXXRecordDecl>(RecordTy->getDecl());
----------------
malcolm.parsons wrote:
> aaron.ballman wrote:
> > You can use `auto` here and below.
> This was copied from `CXXRecordDecl::addedMember()`.
Not all LLVM/Clang code is ideal. It makes sense to clean up issues once the code is changed and even more so when it's copied elsewhere.


https://reviews.llvm.org/D38179