angelgarcia updated this revision to Diff 38648.
angelgarcia added a comment.
Small tweak.
http://reviews.llvm.org/D14145
Files:
clang-tidy/modernize/UseDefaultCheck.cpp
test/clang-tidy/modernize-use-default-copy.cpp
test/clang-tidy/modernize-use-default.cpp
Index: test/clang-tidy/modernize-use-default.cpp
===================================================================
--- test/clang-tidy/modernize-use-default.cpp
+++ test/clang-tidy/modernize-use-default.cpp
@@ -135,3 +135,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
// CHECK-FIXES: ~N() override = default;
};
+
+struct O {
+ O() {
+ // Don't erase comments inside the body.
+ }
+ ~O() {
+ // Don't erase comments inside the body.
+ }
+};
+
Index: test/clang-tidy/modernize-use-default-copy.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-default-copy.cpp
@@ -0,0 +1,422 @@
+// RUN: %check_clang_tidy %s modernize-use-default %t -- -- -std=c++11 -fno-delayed-template-parsing
+
+// 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-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-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-FIXES: IL &operator=(const IL &Other) = default;
+ int Field;
+};
+
+// Wrong type.
+struct WT {
+ WT(const IL &Other) {}
+ WT &operator=(const IL &);
+};
+WT &WT::operator=(const IL &Other) { return *this; }
+
+// Qualifiers.
+struct Qual {
+ Qual(const Qual &Other) : Field(Other.Field), Volatile(Other.Volatile),
+ Mutable(Other.Mutable), Reference(Other.Reference),
+ Const(Other.Const) {}
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use '= default'
+ // CHECK-FIXES: Qual(const Qual &Other) = default;
+
+ int Field;
+ volatile char Volatile;
+ mutable bool Mutable;
+ const OL &Reference; // This makes this class non-assignable.
+ const IL Const; // This also makes this class non-assignable.
+ static int Static;
+};
+
+// Wrong init arguments.
+struct WI {
+ WI(const WI &Other) : Field1(Other.Field1), Field2(Other.Field1) {}
+ WI &operator=(const WI &);
+ int Field1, Field2;
+};
+WI &WI::operator=(const WI &Other) {
+ Field1 = Other.Field1;
+ Field2 = Other.Field1;
+ return *this;
+}
+
+// Missing field.
+struct MF {
+ MF(const MF &Other) : Field1(Other.Field1), Field2(Other.Field2) {}
+ MF &operator=(const MF &);
+ int Field1, Field2, Field3;
+};
+MF &MF::operator=(const MF &Other) {
+ Field1 = Other.Field1;
+ Field2 = Other.Field2;
+ return *this;
+}
+
+struct Comments {
+ Comments(const Comments &Other)
+ /* don't delete */ : /* this comment */ Field(Other.Field) {}
+ int Field;
+};
+
+struct MoreComments {
+ MoreComments(const MoreComments &Other) /* this comment is OK */
+ : Field(Other.Field) {}
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+ // CHECK-FIXES: MoreComments(const MoreComments &Other) /* this comment is OK */
+ // CHECK-FIXES-NEXT: = default;
+ int Field;
+};
+
+struct ColonInComment {
+ ColonInComment(const ColonInComment &Other) /* : */ : Field(Other.Field) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+ // CHECK-FIXES: ColonInComment(const ColonInComment &Other) /* : */ = default;
+ int Field;
+};
+
+// 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-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'
+ // CHECK-FIXES: BF(const BF &Other) = default;
+ BF &operator=(const BF &);
+
+ unsigned Field1 : 3;
+ int : 7;
+ char Field2 : 6;
+ int : 0;
+ int Field3 : 24;
+ unsigned char Field4;
+};
+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-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-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-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) {
+ IL::operator=(Other);
+ OL::operator=(Other);
+ return *this;
+}
+
+// Base classes, incorrect parameter.
+struct BCIP : BCWM, BF {
+ BCIP(const BCIP &Other) : BCWM(Other), BF(Other.Bf) {}
+ BCIP &operator=(const BCIP &);
+};
+BCIP &BCIP::operator=(const BCIP &Other) {
+ BCWM::operator=(Other);
+ BF::operator=(Other.Bf);
+ return *this;
+}
+
+// Virtual base classes.
+struct VA : virtual OL {};
+struct VB : virtual OL {};
+struct VBC : VA, VB, virtual OL {
+ // OL is the first thing that is going to be initialized, despite the fact
+ // that it is the last in the list of bases, because it is virtual and there
+ // is a virtual OL at the beginning of VA (which is the same).
+ VBC(const VBC &Other) : OL(Other), VA(Other), VB(Other) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+ // CHECK-FIXES: VBC(const VBC &Other) = default;
+ 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-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) {
+ OL::operator=(Other);
+ VBC::operator=(Other);
+ return *this;
+}
+
+// Class template.
+template <class T>
+struct Template {
+ Template() = default;
+ Template(const Template &Other) : Field(Other.Field) {}
+ Template &operator=(const Template &Other);
+ void foo(const T &t);
+ int Field;
+};
+template <class T>
+Template<T> &Template<T>::operator=(const Template<T> &Other) {
+ Field = Other.Field;
+ return *this;
+}
+Template<int> T1;
+
+// Dependent types.
+template <class T>
+struct DT1 {
+ DT1() = default;
+ DT1(const DT1 &Other) : Field(Other.Field) {}
+ DT1 &operator=(const DT1 &);
+ T Field;
+};
+template <class T>
+DT1<T> &DT1<T>::operator=(const DT1<T> &Other) {
+ Field = Other.Field;
+ return *this;
+}
+DT1<int> Dt1;
+
+template <class T>
+struct DT2 {
+ DT2() = default;
+ DT2(const DT2 &Other) : Field(Other.Field), Dependent(Other.Dependent) {}
+ DT2 &operator=(const DT2 &);
+ T Field;
+ typename T::TT Dependent;
+};
+template <class T>
+DT2<T> &DT2<T>::operator=(const DT2<T> &Other) {
+ Field = Other.Field;
+ Dependent = Other.Dependent;
+ return *this;
+}
+struct T {
+ typedef int TT;
+};
+DT2<T> Dt2;
+
+// Default arguments.
+struct DA {
+ DA(int Int);
+ DA(const DA &Other = DA(0)) : Field1(Other.Field1), Field2(Other.Field2) {}
+ DA &operator=(const DA &);
+ int Field1;
+ 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-FIXES: DA &DA::operator=(const DA &Other) = default;
+
+// Default initialization.
+struct DI {
+ DI(const DI &Other) : Field1(Other.Field1), Field2(Other.Field2) {}
+ int Field1;
+ int Field2 = 0;
+ int Fiedl3;
+};
+
+// Statement inside body.
+void foo();
+struct SIB {
+ SIB(const SIB &Other) : Field(Other.Field) { foo(); }
+ SIB &operator=(const SIB &);
+ int Field;
+};
+SIB &SIB::operator=(const SIB &Other) {
+ Field = Other.Field;
+ foo();
+ return *this;
+}
+
+// Comment inside body.
+struct CIB {
+ CIB(const CIB &Other) : Field(Other.Field) { /* Don't erase this */
+ }
+ 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-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-FIXES: NCRef &NCRef::operator=(NCRef &Other) = default;
+
+// Already defaulted.
+struct IAD {
+ IAD(const IAD &Other) = default;
+ IAD &operator=(const IAD &Other) = default;
+};
+
+struct OAD {
+ OAD(const OAD &Other);
+ OAD &operator=(const OAD &);
+};
+OAD::OAD(const OAD &Other) = default;
+OAD &OAD::operator=(const OAD &Other) = default;
+
+// Deleted.
+struct ID {
+ ID(const ID &Other) = delete;
+ ID &operator=(const ID &Other) = delete;
+};
+
+// Non-reference parameter.
+struct NRef {
+ NRef &operator=(NRef Other);
+ int Field1;
+};
+NRef &NRef::operator=(NRef Other) {
+ Field1 = Other.Field1;
+ return *this;
+}
+
+// RValue reference parameter.
+struct RVR {
+ RVR(RVR &&Other) {}
+ RVR &operator=(RVR &&);
+};
+RVR &RVR::operator=(RVR &&Other) { return *this; }
+
+// Similar function.
+struct SF {
+ SF &foo(const SF &);
+ int Field1;
+};
+SF &SF::foo(const SF &Other) {
+ Field1 = Other.Field1;
+ return *this;
+}
+
+// No return.
+struct NR {
+ NR &operator=(const NR &);
+};
+NR &NR::operator=(const NR &Other) {}
+
+// Return misplaced.
+struct RM {
+ RM &operator=(const RM &);
+ int Field;
+};
+RM &RM::operator=(const RM &Other) {
+ return *this;
+ Field = Other.Field;
+}
+
+// Wrong return value.
+struct WRV {
+ WRV &operator=(WRV &);
+};
+WRV &WRV::operator=(WRV &Other) {
+ return Other;
+}
+
+// Wrong return type.
+struct WRT : IL {
+ IL &operator=(const WRT &);
+};
+IL &WRT::operator=(const WRT &Other) {
+ return *this;
+}
+
Index: clang-tidy/modernize/UseDefaultCheck.cpp
===================================================================
--- clang-tidy/modernize/UseDefaultCheck.cpp
+++ clang-tidy/modernize/UseDefaultCheck.cpp
@@ -10,52 +10,355 @@
#include "UseDefaultCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
namespace clang {
namespace tidy {
namespace modernize {
-static const char CtorDtor[] = "CtorDtorDecl";
+static const char SpecialFunction[] = "SpecialFunction";
+
+/// \brief Finds the SourceLocation of the colon ':' before the initialization
+/// list in the definition of a constructor.
+static SourceLocation getColonLoc(const ASTContext *Context,
+ const CXXConstructorDecl *Ctor) {
+ // FIXME: First init is the first initialization that is going to be
+ // performed, no matter what was the real order in the source code. If the
+ // order of the inits is wrong in the code, it may result in a false negative.
+ SourceLocation FirstInit = (*Ctor->init_begin())->getSourceLocation();
+ SourceLocation LastArg =
+ Ctor->getParamDecl(Ctor->getNumParams() - 1)->getLocEnd();
+ // We need to find the colon between the ')' and the first initializer.
+ bool Invalid = false;
+ StringRef Text = Lexer::getSourceText(
+ CharSourceRange::getCharRange(LastArg, FirstInit),
+ Context->getSourceManager(), Context->getLangOpts(), &Invalid);
+ if (Invalid)
+ return SourceLocation();
+
+ size_t ColonPos = Text.rfind(':');
+ if (ColonPos == StringRef::npos)
+ return SourceLocation();
+
+ Text = Text.drop_front(ColonPos + 1);
+ if (std::strspn(Text.data(), " \t\r\n") != Text.size()) {
+ // If there are comments, preprocessor directives or anything, abort.
+ return SourceLocation();
+ }
+ // FIXME: don't remove comments in the middle of the initializers.
+ return LastArg.getLocWithOffset(ColonPos);
+}
+
+/// \brief Returns true if \p E is a DeclRefExpr to the variable declared with
+/// \p Var.
+static bool exprRefersToVariable(const Expr *E, const ValueDecl *Var) {
+ const auto *DeclRef = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts());
+ return DeclRef && DeclRef->getDecl() &&
+ DeclRef->getDecl()->getCanonicalDecl() == Var->getCanonicalDecl();
+}
+
+/// \brief Returns true if \p E is a member expression, where the base is the
+/// variable declared with \p Var and the accessed member is the one declared
+/// as \p Field.
+///
+/// FIXME: always returns false in class templates.
+static bool exprIsAccessToFieldInVar(const Expr *E, const FieldDecl *Field,
+ const ValueDecl *Var) {
+ // Ensure that the expression is a member expression.
+ const auto *Access = dyn_cast<MemberExpr>(E->IgnoreParenImpCasts());
+ if (!Access)
+ return false;
+
+ // It must be an access to the specified field.
+ if (dyn_cast<FieldDecl>(Access->getMemberDecl()) != Field)
+ return false;
+
+ // The base must be a DeclRefExpr to Var.
+ return exprRefersToVariable(Access->getBase(), Var);
+}
+
+/// \brief Finds all the named non-static fields of \p Record.
+static std::set<const FieldDecl *>
+getAllNamedFields(const CXXRecordDecl *Record) {
+ std::set<const FieldDecl *> Result;
+ for (const auto *Field : Record->fields()) {
+ // Static data members are not in this range.
+ if (Field->isUnnamedBitfield())
+ continue;
+ Result.insert(Field);
+ }
+ return Result;
+}
+
+/// \brief Returns the names of the direct bases of \p Record, both virtual and
+/// non-virtual.
+static std::set<const Type *> getAllDirectBases(const CXXRecordDecl *Record) {
+ std::set<const Type *> Result;
+ for (auto Base : Record->bases()) {
+ // CXXBaseSpecifier.
+ const auto *BaseType = Base.getTypeSourceInfo()->getType().getTypePtr();
+ Result.insert(BaseType);
+ }
+ return Result;
+}
+
+/// \brief Checks that the given method receives as only parameter a lvalue
+/// reference to a variable with the type that has this method as a member.
+static bool hasCopySignature(const ASTContext *Context,
+ const CXXMethodDecl *Method) {
+ if (Method->getNumParams() != 1)
+ return false;
+ QualType ArgType = Method->getParamDecl(0)->getType();
+
+ const auto *Record = Method->getParent();
+ QualType RecordType = Context->getTypeDeclType(Record);
+
+ return !ArgType.isNull() && ArgType->isLValueReferenceType() &&
+ ArgType.getNonReferenceType().getUnqualifiedType() == RecordType;
+}
+
+/// \brief Check that the given constructor has copy signature and that it
+/// copy-initializes all its bases and members.
+static bool isCopyConstructorAndCanBeDefaulted(const ASTContext *Context,
+ const CXXConstructorDecl *Ctor) {
+ // An explicitly-defaulted constructor cannot have default arguments.
+ if (!hasCopySignature(Context, Ctor) || Ctor->getMinRequiredArguments() != 1)
+ return false;
+
+ // Base classes and members that have to be copied.
+ const auto *Record = Ctor->getParent();
+ auto BasesToInit = getAllDirectBases(Record);
+ auto FieldsToInit = getAllNamedFields(Record);
+
+ for (const CXXCtorInitializer *Initializer : Ctor->inits()) {
+ if (Initializer->isIndirectMemberInitializer() ||
+ Initializer->isInClassMemberInitializer() ||
+ Initializer->isDelegatingInitializer())
+ return false;
+
+ const auto *Init = Initializer->getInit();
+ if (Initializer->isBaseInitializer()) {
+ // Initialization of a base class. It should be a call to a copy
+ // constructor of the base.
+ const auto *Construct = dyn_cast<CXXConstructExpr>(Init);
+ if (!BasesToInit.count(Initializer->getBaseClass()) || !Construct ||
+ !Construct->getConstructor()->isCopyConstructor() ||
+ !exprRefersToVariable(Construct->getArg(0), Ctor->getParamDecl(0)))
+ return false;
+ BasesToInit.erase(Initializer->getBaseClass());
+ } else if (Initializer->isMemberInitializer()) {
+ const auto *Field = Initializer->getMember();
+ // The initialization is a CXXConstructExpr only for class types.
+ if (const auto *Construct = dyn_cast<CXXConstructExpr>(Init)) {
+ if (!Construct->getConstructor()->isCopyConstructor())
+ return false;
+ Init = Construct->getArg(0);
+ }
+ if (!FieldsToInit.count(Field) ||
+ !exprIsAccessToFieldInVar(Init, Field, Ctor->getParamDecl(0))) {
+ return false;
+ }
+ FieldsToInit.erase(Field);
+ } else {
+ llvm_unreachable("Unknown CXXCtorInitializer");
+ }
+ }
+ return BasesToInit.empty() && FieldsToInit.empty();
+}
+
+/// \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.
+static bool isCopyAssignmentAndCanBeDefaulted(const ASTContext *Context,
+ const CXXMethodDecl *Operator) {
+ if (!Operator->isCopyAssignmentOperator() ||
+ !hasCopySignature(Context, Operator))
+ return false;
+
+ const auto *Record = Operator->getParent();
+ auto BasesToInit = getAllDirectBases(Record);
+ auto FieldsToInit = getAllNamedFields(Record);
+ bool ReturnsThis = false;
+
+ for (const auto *Stmt : cast<CompoundStmt>(Operator->getBody())->body()) {
+ if (const auto *Call = dyn_cast<CXXMemberCallExpr>(Stmt)) {
+ const auto *Object = Call->getImplicitObjectArgument();
+ const auto *Method = Call->getMethodDecl();
+ if (!Object || !Method)
+ return false;
+ // Assignment operator of a base class:
+ // Base::operator=(Other);
+ //
+ // Clang translates this into:
+ // ((Base*)this)->operator=((Base)Other);
+ //
+ // So we are looking for a member call that fulfills:
+
+ // - The object is an implicit cast of 'this' to a pointer to a base
+ // class.
+ const auto *Cast = cast<ImplicitCastExpr>(Object);
+ if (!Cast || Cast->getCastKind() != CK_UncheckedDerivedToBase ||
+ !Cast->getSubExpr()->isImplicitCXXThis())
+ return false;
+ // - The called method is the operator=.
+ if (!Method->isCopyAssignmentOperator())
+ return false;
+ // - The argument is (an implicit cast to a Base of) the argument taken by
+ // "Operator".
+ if (Call->getNumArgs() != 1 ||
+ !exprRefersToVariable(Call->getArg(0), Operator->getParamDecl(0)))
+ return false;
+
+ // Get the type of the base class.
+ const Type *Base = Cast->getType()->getPointeeType().getTypePtrOrNull();
+ if (!BasesToInit.count(Base))
+ return false;
+ BasesToInit.erase(Base);
+ } else if (const auto *Return = dyn_cast<ReturnStmt>(Stmt)) {
+ // Returns statement of the assignment operator. Has to be:
+ // return *this;
+ const auto *Deref = dyn_cast<UnaryOperator>(Return->getRetValue());
+ if (!Deref || Deref->getOpcode() != UO_Deref ||
+ !isa<CXXThisExpr>(Deref->getSubExpr()))
+ return false;
+ ReturnsThis = true;
+ break;
+ } else {
+ // The assignment of members:
+ // Field = Other.Field;
+ // Is a BinaryOperator in non-class types, and a CXXOperatorCallExpr
+ // otherwise.
+ const auto *BinOp = dyn_cast<BinaryOperator>(Stmt);
+ const auto *OpCall = dyn_cast<CXXOperatorCallExpr>(Stmt);
+ const Expr *LHS, *RHS;
+ if (BinOp && BinOp->getOpcode() == BO_Assign) {
+ LHS = BinOp->getLHS();
+ RHS = BinOp->getRHS();
+ } else if (OpCall && OpCall->getOperator() == OO_Equal) {
+ assert(OpCall->getNumArgs() == 2 &&
+ "operator= doesn't receive 2 arguments");
+ LHS = OpCall->getArg(0);
+ RHS = OpCall->getArg(1);
+ } else {
+ // Unknown statement.
+ return false;
+ }
+ // Left side has to be a data member of 'this'.
+ const auto *Member = dyn_cast<MemberExpr>(LHS);
+ if (!Member || !Member->isImplicitAccess())
+ return false;
+ const auto *Field = dyn_cast<FieldDecl>(Member->getMemberDecl());
+ // getMemberDecl() will return either a FieldDecl or a CXXMethodDecl.
+ if (!Field)
+ return false;
+
+ // Right side has to be an access to the same field in the parameter.
+ if (!FieldsToInit.count(Field) ||
+ !exprIsAccessToFieldInVar(RHS, Field, Operator->getParamDecl(0)))
+ return false;
+ FieldsToInit.erase(Field);
+ }
+ }
+
+ return BasesToInit.empty() && FieldsToInit.empty() && ReturnsThis;
+}
+
+/// \brief Returns false if the body has any non-whitespace character.
+static bool bodyEmpty(const ASTContext *Context, const CompoundStmt *Body) {
+ bool Invalid = false;
+ StringRef Text = Lexer::getSourceText(
+ CharSourceRange::getCharRange(Body->getLBracLoc().getLocWithOffset(1),
+ Body->getRBracLoc()),
+ Context->getSourceManager(), Context->getLangOpts(), &Invalid);
+ return !Invalid && std::strspn(Text.data(), " \t\r\n") == Text.size();
+}
+
+/// \brief Matcher for method declarations that are overloading the
+/// copy-assignment operator.
+AST_MATCHER(CXXMethodDecl, isCopyAssignmentOperator) {
+ return Node.isCopyAssignmentOperator();
+}
void UseDefaultCheck::registerMatchers(MatchFinder *Finder) {
if (getLangOpts().CPlusPlus) {
+ // Destructor.
+ Finder->addMatcher(cxxDestructorDecl(isDefinition()).bind(SpecialFunction),
+ this);
Finder->addMatcher(
- cxxConstructorDecl(isDefinition(),
- unless(hasAnyConstructorInitializer(anything())),
- parameterCountIs(0))
- .bind(CtorDtor),
+ cxxConstructorDecl(
+ isDefinition(),
+ anyOf(
+ // Default constructor.
+ allOf(unless(hasAnyConstructorInitializer(anything())),
+ parameterCountIs(0)),
+ // Copy constructor.
+ allOf(parameterCountIs(1),
+ hasParameter(0, hasType(lValueReferenceType())))))
+ .bind(SpecialFunction),
this);
- Finder->addMatcher(cxxDestructorDecl(isDefinition()).bind(CtorDtor), this);
+ // Copy-assignment operator.
+ Finder->addMatcher(cxxMethodDecl(isDefinition(), isCopyAssignmentOperator())
+ .bind(SpecialFunction),
+ this);
}
}
void UseDefaultCheck::check(const MatchFinder::MatchResult &Result) {
+ std::string SpecialFunctionName;
+ SourceLocation StartLoc, EndLoc;
+
// Both CXXConstructorDecl and CXXDestructorDecl inherit from CXXMethodDecl.
- const auto *CtorDtorDecl = Result.Nodes.getNodeAs<CXXMethodDecl>(CtorDtor);
+ const auto *SpecialFunctionDecl =
+ Result.Nodes.getNodeAs<CXXMethodDecl>(SpecialFunction);
+
+ // Discard explicitly deleted/defaulted special member functions and those
+ // that are not user-provided (automatically generated).
+ if (SpecialFunctionDecl->isDeleted() ||
+ SpecialFunctionDecl->isExplicitlyDefaulted() ||
+ !SpecialFunctionDecl->isUserProvided() || !SpecialFunctionDecl->hasBody())
+ return;
+
+ const auto *Body = dyn_cast<CompoundStmt>(SpecialFunctionDecl->getBody());
+ if (!Body)
+ return;
- // Discard explicitly deleted/defaulted constructors/destructors, those that
- // are not user-provided (automatically generated constructor/destructor), and
- // those with non-empty bodies.
- if (CtorDtorDecl->isDeleted() || CtorDtorDecl->isExplicitlyDefaulted() ||
- !CtorDtorDecl->isUserProvided() || !CtorDtorDecl->hasTrivialBody())
+ // Default locations.
+ StartLoc = Body->getLBracLoc();
+ EndLoc = Body->getRBracLoc();
+
+ // If there are comments inside the body, don't do the change.
+ if (!SpecialFunctionDecl->isCopyAssignmentOperator() &&
+ !bodyEmpty(Result.Context, Body))
return;
- const auto *Body = dyn_cast<CompoundStmt>(CtorDtorDecl->getBody());
- // This should never happen, since 'hasTrivialBody' checks that this is
- // actually a CompoundStmt.
- assert(Body && "Definition body is not a CompoundStmt");
-
- diag(CtorDtorDecl->getLocStart(),
- "use '= default' to define a trivial " +
- std::string(dyn_cast<CXXConstructorDecl>(CtorDtorDecl)
- ? "default constructor"
- : "destructor"))
+ 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.
+ if (Ctor->getNumCtorInitializers() != 0) {
+ StartLoc = getColonLoc(Result.Context, Ctor);
+ if (!StartLoc.isValid())
+ return;
+ }
+ } else if (dyn_cast<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(
- CharSourceRange::getTokenRange(Body->getLBracLoc(),
- Body->getRBracLoc()),
- "= default;");
+ CharSourceRange::getTokenRange(StartLoc, EndLoc), "= default;");
}
} // namespace modernize
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
