[clang-tidy] Changes to modernize-use-default check

[malcolm.parsons]

Summary

Warn about special member functions that only contain a comment.
Report the location of the special member function, unless it is
defined in a macro. Reporting the location of the body in a macro is
more helpful as it causes the macro expansion location to be reported too.

Fixes PR30920.

[aaron.ballman]

LGTM!

clang-tidy/modernize/UseDefaultCheck.cpp

if (SpecialFunctionDecl->isDeleted() ||
       SpecialFunctionDecl->isTemplateInstantiation() ||
       !SpecialFunctionDecl->isUserProvided() || !SpecialFunctionDecl->hasBody())
     return;
 
   const auto *Body = dyn_cast<CompoundStmt>(SpecialFunctionDecl->getBody());
   if (!Body)
     return;
 
-  // If there are comments inside the body, don't do the change.
-  if (!SpecialFunctionDecl->isCopyAssignmentOperator() &&
-      !bodyEmpty(Result.Context, Body))
+  // If there is code inside the body, don't warn.
+  if (!SpecialFunctionDecl->isCopyAssignmentOperator() && !Body->body_empty())
     return;
 
+  // If there are comments inside the body, don't do the change.
+  bool ApplyFix = SpecialFunctionDecl->isCopyAssignmentOperator() ||
+                  bodyEmpty(Result.Context, Body);
+
   std::vector<FixItHint> RemoveInitializers;
 
   if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(SpecialFunctionDecl)) {
     if (Ctor->getNumParams() == 0) {
       SpecialFunctionName = "default constructor";
     } else {
       if (!isCopyConstructorAndCanBeDefaulted(Result.Context, Ctor))
         return;
       SpecialFunctionName = "copy constructor";
       // If there are constructor initializers, they must be removed.
       for (const auto *Init : Ctor->inits()) {
         RemoveInitializers.emplace_back(
             FixItHint::CreateRemoval(Init->getSourceRange()));
       }
     }
   } else if (isa<CXXDestructorDecl>(SpecialFunctionDecl)) {
     SpecialFunctionName = "destructor";
   } else {
     if (!isCopyAssignmentAndCanBeDefaulted(Result.Context, SpecialFunctionDecl))
       return;
     SpecialFunctionName = "copy-assignment operator";
   }
 
-  diag(SpecialFunctionDecl->getLocStart(),
-       "use '= default' to define a trivial " + SpecialFunctionName)
-      << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+  // The location of the body is more useful inside a macro as spelling and
+  // expansion locations are reported.
+  SourceLocation Location = SpecialFunctionDecl->getLocation();
+  if (Location.isMacroID())
+    Location = Body->getLocStart();
+
+  auto Diag = diag(Location, "use '= default' to define a trivial " +
+                                 SpecialFunctionName);
+
+  if (ApplyFix)
+    Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
          << RemoveInitializers;
 }
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

test/clang-tidy/modernize-use-default-copy.cpp

 // RUN: %check_clang_tidy %s modernize-use-default %t -- -- -std=c++11 -fno-delayed-template-parsing -fexceptions
 
 // Out of line definition.
 struct OL {
   OL(const OL &);
   OL &operator=(const OL &);
   int Field;
 };
 OL::OL(const OL &Other) : Field(Other.Field) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial copy constructor [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial copy constructor [modernize-use-default]
 // CHECK-FIXES: OL::OL(const OL &Other)  = default;
 OL &OL::operator=(const OL &Other) {
   Field = Other.Field;
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-4]]:1: warning: use '= default' to define a trivial copy-assignment operator [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: use '= default' to define a trivial copy-assignment operator [modernize-use-default]
 // CHECK-FIXES: OL &OL::operator=(const OL &Other) = default;
 
 // Inline.
 struct IL {
   IL(const IL &Other) : Field(Other.Field) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: IL(const IL &Other)  = default;
   IL &operator=(const IL &Other) {
     Field = Other.Field;
     return *this;
   }
-  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use '= default'
+  // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: use '= default'
   // CHECK-FIXES: IL &operator=(const IL &Other) = default;
   int Field;
 };
 
 // Wrong type.
 struct WT {
   WT(const IL &Other) {}
   WT &operator=(const IL &);

 // No members or bases (in particular, no colon).
 struct Empty {
   Empty(const Empty &Other) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: Empty(const Empty &Other) = default;
   Empty &operator=(const Empty &);
 };
 Empty &Empty::operator=(const Empty &Other) { return *this; }
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use '= default'
 // CHECK-FIXES: Empty &Empty::operator=(const Empty &Other) = default;
 
 // Bit fields.
 struct BF {
   BF() = default;
   BF(const BF &Other) : Field1(Other.Field1), Field2(Other.Field2), Field3(Other.Field3),
                         Field4(Other.Field4) {}
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'

 };
 BF &BF::operator=(const BF &Other) {
   Field1 = Other.Field1;
   Field2 = Other.Field2;
   Field3 = Other.Field3;
   Field4 = Other.Field4;
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-7]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-7]]:9: warning: use '= default'
 // CHECK-FIXES: BF &BF::operator=(const BF &Other) = default;
 
 // Base classes.
 struct BC : IL, OL, BF {
   BC(const BC &Other) : IL(Other), OL(Other), BF(Other) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: BC(const BC &Other)  = default;
   BC &operator=(const BC &Other);
 };
 BC &BC::operator=(const BC &Other) {
   IL::operator=(Other);
   OL::operator=(Other);
   BF::operator=(Other);
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-6]]:9: warning: use '= default'
 // CHECK-FIXES: BC &BC::operator=(const BC &Other) = default;
 
 // Base classes with member.
 struct BCWM : IL, OL {
   BCWM(const BCWM &Other) : IL(Other), OL(Other), Bf(Other.Bf) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: BCWM(const BCWM &Other)  = default;
   BCWM &operator=(const BCWM &);
   BF Bf;
 };
 BCWM &BCWM::operator=(const BCWM &Other) {
   IL::operator=(Other);
   OL::operator=(Other);
   Bf = Other.Bf;
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-6]]:13: warning: use '= default'
 // CHECK-FIXES: BCWM &BCWM::operator=(const BCWM &Other) = default;
 
 // Missing base class.
 struct MBC : IL, OL, BF {
   MBC(const MBC &Other) : IL(Other), OL(Other) {}
   MBC &operator=(const MBC &);
 };
 MBC &MBC::operator=(const MBC &Other) {
struct VBC : VA, VB, virtual OL {
   VBC &operator=(const VBC &Other);
 };
 VBC &VBC::operator=(const VBC &Other) {
   OL::operator=(Other);
   VA::operator=(Other);
   VB::operator=(Other);
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-6]]:11: warning: use '= default'
 // CHECK-FIXES: VBC &VBC::operator=(const VBC &Other) = default;
 
 // Indirect base.
 struct IB : VBC {
   IB(const IB &Other) : OL(Other), VBC(Other) {}
   IB &operator=(const IB &);
 };
 IB &IB::operator=(const IB &Other) {
struct DA {
   char Field2;
 };
 // Overloaded operator= cannot have a default argument.
 DA &DA::operator=(const DA &Other) {
   Field1 = Other.Field1;
   Field2 = Other.Field2;
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-5]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: use '= default'
 // CHECK-FIXES: DA &DA::operator=(const DA &Other) = default;
 
 struct DA2 {
   // Can be used as copy-constructor but cannot be explicitly defaulted.
   DA2(const DA &Other, int Def = 0) {}
 };
 
 // Default initialization.
SIB &SIB::operator=(const SIB &Other) {
   foo();
   return *this;
 }
 
 // Comment inside body.
 struct CIB {
   CIB(const CIB &Other) : Field(Other.Field) { /* Don't erase this */
   }
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
   CIB &operator=(const CIB &);
   int Field;
 };
 CIB &CIB::operator=(const CIB &Other) {
   Field = Other.Field;
   // FIXME: don't erase this comment.
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-5]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-5]]:11: warning: use '= default'
 // CHECK-FIXES: CIB &CIB::operator=(const CIB &Other) = default;
 
 // Take non-const reference as argument.
 struct NCRef {
   NCRef(NCRef &Other) : Field1(Other.Field1), Field2(Other.Field2) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: NCRef(NCRef &Other)  = default;
   NCRef &operator=(NCRef &);
   int Field1, Field2;
 };
 NCRef &NCRef::operator=(NCRef &Other) {
   Field1 = Other.Field1;
   Field2 = Other.Field2;
   return *this;
 }
-// CHECK-MESSAGES: :[[@LINE-5]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-5]]:15: warning: use '= default'
 // CHECK-FIXES: NCRef &NCRef::operator=(NCRef &Other) = default;
 
 // Already defaulted.
 struct IAD {
   IAD(const IAD &Other) = default;
   IAD &operator=(const IAD &Other) = default;
 };
 

 
 // noexcept(false).
 struct NEF {
   // This is the default.
   //NEF(const NEF &Other) noexcept(false) {}
   NEF &operator=(const NEF &Other) noexcept(false);
 };
 //NEF &NEF::operator=(const NEF &Other) noexcept(false) { return *this; }
+
+#define STRUCT_WITH_COPY_CONSTRUCT(_base, _type) \
+  struct _type {                                 \
+    _type(const _type &v) : value(v.value) {}    \
+    _base value;                                 \
+  };
+
+STRUCT_WITH_COPY_CONSTRUCT(unsigned char, Hex8CopyConstruct)
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial copy constructor
+// CHECK-MESSAGES: :[[@LINE-6]]:44: note:
+
+#define STRUCT_WITH_COPY_ASSIGN(_base, _type) \
+  struct _type {                              \
+    _type &operator=(const _type &rhs) {      \
+      value = rhs.value;                      \
+      return *this;                           \
+    }                                         \
+    _base value;                              \
+  };
+
+STRUCT_WITH_COPY_ASSIGN(unsigned char, Hex8CopyAssign)
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial copy-assignment operator
+// CHECK-MESSAGES: :[[@LINE-9]]:40: note:

test/clang-tidy/modernize-use-default.cpp

 // RUN: %check_clang_tidy %s modernize-use-default %t -- -- -std=c++11 -fno-delayed-template-parsing  -fexceptions
 
 // Out of line definition.
 class OL {
 public:
   OL();
   ~OL();
 };
 
 OL::OL() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial default constructor [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial default constructor [modernize-use-default]
 // CHECK-FIXES: OL::OL() = default;
 OL::~OL() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial destructor [modernize-use-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial destructor [modernize-use-default]
 // CHECK-FIXES: OL::~OL() = default;
 
 // Inline definitions.
 class IL {
 public:
   IL() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: IL() = default;
public:
   Del() = delete;
   ~Del() = delete;
 };
 
 // Do not remove other keywords.
 class KW {
 public:
   explicit KW() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use '= default'
   // CHECK-FIXES: explicit KW() = default;
   virtual ~KW() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use '= default'
   // CHECK-FIXES: virtual ~KW() = default;
 };
 
 // Nested class.
 struct N {
   struct NN {
     NN() {}
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default'

 class TempODef {
 public:
   TempODef();
   ~TempODef();
 };
 
 template <class T>
 TempODef<T>::TempODef() {}
-// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use '= default'
 // CHECK-FIXES: TempODef<T>::TempODef() = default;
 template <class T>
 TempODef<T>::~TempODef() {}
-// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use '= default'
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use '= default'
 // CHECK-FIXES: TempODef<T>::~TempODef() = default;
 
 template class TempODef<int>;
 template class TempODef<double>;
 
 // Non user-provided constructor/destructor.
 struct Imp {
   int Int;
struct DC : KW {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~DC() override = default;
 };
 
 struct Comments {
   Comments() {
     // Don't erase comments inside the body.
   }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use '= default'
   ~Comments() {
     // Don't erase comments inside the body.
   }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use '= default'
 };
 
 // Try-catch.
 struct ITC {
   ITC() try {} catch(...) {}
   ~ITC() try {} catch(...) {}
 };
 
 struct OTC {
   OTC();
   ~OTC();
 };
 OTC::OTC() try {} catch(...) {}
 OTC::~OTC() try {} catch(...) {}
+
+#define STRUCT_WITH_DEFAULT(_base, _type) \
+  struct _type {                          \
+    _type() {}                            \
+    _base value;                          \
+  };
+
+STRUCT_WITH_DEFAULT(unsigned char, Hex8Default)
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial default constructor
+// CHECK-MESSAGES: :[[@LINE-6]]:13: note: