Index: clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultCheck.cpp @@ -249,11 +249,14 @@ 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 RemoveInitializers; if (const auto *Ctor = dyn_cast(SpecialFunctionDecl)) { @@ -277,10 +280,18 @@ SpecialFunctionName = "copy-assignment operator"; } - diag(SpecialFunctionDecl->getLocStart(), - "use '= default' to define a trivial " + SpecialFunctionName) - << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;") - << RemoveInitializers; + // 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 Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-copy.cpp @@ -7,13 +7,13 @@ 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. @@ -25,7 +25,7 @@ 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; }; @@ -110,7 +110,7 @@ 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. @@ -137,7 +137,7 @@ 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. @@ -153,7 +153,7 @@ 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. @@ -170,7 +170,7 @@ 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. @@ -213,7 +213,7 @@ 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. @@ -291,7 +291,7 @@ 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 { @@ -324,6 +324,7 @@ 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; }; @@ -332,7 +333,7 @@ // 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. @@ -348,7 +349,7 @@ 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. @@ -471,3 +472,26 @@ 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: Index: clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-default.cpp @@ -8,10 +8,10 @@ }; 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. @@ -92,10 +92,10 @@ 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; }; @@ -134,11 +134,11 @@ template TempODef::TempODef() {} -// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use '= default' +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use '= default' // CHECK-FIXES: TempODef::TempODef() = default; template TempODef::~TempODef() {} -// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use '= default' +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use '= default' // CHECK-FIXES: TempODef::~TempODef() = default; template class TempODef; @@ -178,9 +178,11 @@ 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. @@ -195,3 +197,13 @@ }; 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: