Index: clang-tidy/modernize/UseDefaultMemberInitCheck.h =================================================================== --- clang-tidy/modernize/UseDefaultMemberInitCheck.h +++ clang-tidy/modernize/UseDefaultMemberInitCheck.h @@ -31,12 +31,18 @@ private: void checkDefaultInit(const ast_matchers::MatchFinder::MatchResult &Result, - const CXXCtorInitializer *Init); + const CXXCtorInitializer *Init, + const CXXConstructorDecl *Ctor); void checkExistingInit(const ast_matchers::MatchFinder::MatchResult &Result, - const CXXCtorInitializer *Init); + const CXXCtorInitializer *Init, + const CXXConstructorDecl *Ctor); + + void removeTrailingColon(DiagnosticBuilder &Diag, const ASTContext *Context, + const CXXConstructorDecl *Ctor); const bool UseAssignment; const bool IgnoreMacros; + std::map RemovedInitializers; }; } // namespace modernize Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp =================================================================== --- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp +++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp @@ -12,12 +12,45 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" +#include "../utils/LexerUtils.h" + using namespace clang::ast_matchers; namespace clang { namespace tidy { namespace modernize { +static SourceLocation findPreviousLocation(const ASTContext *Context, + SourceLocation Location, + tok::TokenKind K) { + for (;;) { + Token Tok = utils::lexer::getPreviousToken(*Context, Location); + SourceLocation TokLoc = Tok.getLocation(); + if (TokLoc.isInvalid() || Tok.is(K)) + return TokLoc; + } +} + +static SourceLocation findColonLocation(const ASTContext *Context, + SourceLocation Location) { + return findPreviousLocation(Context, Location, tok::colon); +} + +static SourceLocation findPreviousCommaLocation(const ASTContext *Context, + SourceLocation Location) { + return findPreviousLocation(Context, Location, tok::comma); +} + +static SourceLocation findInitEndLoc(const CXXCtorInitializer *Init, + const CXXConstructorDecl *Ctor) { + auto pos = std::find(Ctor->init_begin(), Ctor->init_end(), Init); + auto NextInit = std::next(pos); + if (NextInit != Ctor->init_end()) { + return (*NextInit)->getSourceLocation(); + } + return Ctor->getBody()->getLocStart(); +} + static StringRef getValueOfValueInit(const QualType InitType) { switch (InitType->getScalarTypeKind()) { case Type::STK_CPointer: @@ -140,7 +173,8 @@ ClangTidyContext *Context) : ClangTidyCheck(Name, Context), UseAssignment(Options.get("UseAssignment", 0) != 0), - IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true) != 0) {} + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true) != 0), + RemovedInitializers() {} void UseDefaultMemberInitCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { @@ -173,7 +207,8 @@ hasInClassInitializer(anything()), hasParent(recordDecl(isUnion()))))), isWritten(), withInitializer(ignoringImplicit(Init))) - .bind("default"))), + .bind("default"))) + .bind("ctor"), this); Finder->addMatcher( @@ -183,23 +218,26 @@ cxxCtorInitializer(forField(hasInClassInitializer(anything())), isWritten(), withInitializer(ignoringImplicit(Init))) - .bind("existing"))), + .bind("existing"))) + .bind("ctor"), this); } void UseDefaultMemberInitCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Ctor = Result.Nodes.getNodeAs("ctor"); if (const auto *Default = Result.Nodes.getNodeAs("default")) - checkDefaultInit(Result, Default); + checkDefaultInit(Result, Default, Ctor); else if (const auto *Existing = Result.Nodes.getNodeAs("existing")) - checkExistingInit(Result, Existing); + checkExistingInit(Result, Existing, Ctor); else llvm_unreachable("Bad Callback. No node provided."); } void UseDefaultMemberInitCheck::checkDefaultInit( - const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) { + const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init, + const CXXConstructorDecl *Ctor) { const FieldDecl *Field = Init->getAnyMember(); SourceLocation StartLoc = Field->getLocStart(); @@ -227,19 +265,71 @@ if (!UseAssignment) Diag << FixItHint::CreateInsertion(FieldEnd, "}"); - Diag << FixItHint::CreateRemoval(Init->getSourceRange()); + Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange( + Init->getSourceLocation(), findInitEndLoc(Init, Ctor))); + + if (++RemovedInitializers[Ctor] == Ctor->getNumCtorInitializers()) { + removeTrailingColon(Diag, Result.Context, Ctor); + } } void UseDefaultMemberInitCheck::checkExistingInit( - const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init) { + const MatchFinder::MatchResult &Result, const CXXCtorInitializer *Init, + const CXXConstructorDecl *Ctor) { const FieldDecl *Field = Init->getMember(); if (!sameValue(Field->getInClassInitializer(), Init->getInit())) return; - diag(Init->getSourceLocation(), "member initializer for %0 is redundant") - << Field - << FixItHint::CreateRemoval(Init->getSourceRange()); + auto Diag = + diag(Init->getSourceLocation(), "member initializer for %0 is redundant") + << Field; + + auto pos = std::find(Ctor->init_begin(), Ctor->init_end(), Init) - + Ctor->init_begin(); + if (Ctor->getNumCtorInitializers() == 1) { + // Only 1 initializer, removing the SourceRange is fine, the colon will be + // removed later. + Diag << FixItHint::CreateRemoval(Init->getSourceRange()); + } else if (pos == 0) { + // We are removing the 1st initializer and are avoiding to keep ': ,'. + const auto *NextInit = *std::next(Ctor->init_begin()); + if (sameValue(NextInit->getMember()->getInClassInitializer(), + NextInit->getInit())) { + // The next initializer will be removed later. Removing only the + // initializer is enough. Colon will be treated later. + Diag << FixItHint::CreateRemoval(Init->getSourceRange()); + } else { + // The next initializer will not be remove. In this case, we should create + // a removal to the next ',' or the Ctor body. + Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange( + Init->getSourceLocation(), findInitEndLoc(Init, Ctor))); + } + } else { + // This initializer is in the middle of other one. + // In this case, we can create a removal from the previous ',' to the end of + // the initializer. + Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange( + findPreviousCommaLocation(Result.Context, Init->getSourceLocation()), + Init->getRParenLoc().getLocWithOffset(1))); + } + + if (++RemovedInitializers[Ctor] == Ctor->getNumCtorInitializers()) { + removeTrailingColon(Diag, Result.Context, Ctor); + } +} + +void UseDefaultMemberInitCheck::removeTrailingColon( + DiagnosticBuilder &Diag, const ASTContext *Context, + const CXXConstructorDecl *Ctor) { + const CXXCtorInitializer *FirstInit = *Ctor->init_begin(); + SourceLocation ColonLoc = + findColonLocation(Context, FirstInit->getSourceLocation()); + + if (ColonLoc.isInvalid() || ColonLoc.isMacroID()) + return; + + Diag << FixItHint::CreateRemoval(ColonLoc); } } // namespace modernize Index: unittests/clang-tidy/CMakeLists.txt =================================================================== --- unittests/clang-tidy/CMakeLists.txt +++ unittests/clang-tidy/CMakeLists.txt @@ -12,6 +12,7 @@ IncludeInserterTest.cpp GoogleModuleTest.cpp LLVMModuleTest.cpp + ModernizerModuleTest.cpp NamespaceAliaserTest.cpp ObjCModuleTest.cpp OverlappingReplacementsTest.cpp @@ -29,6 +30,7 @@ clangTidyAndroidModule clangTidyGoogleModule clangTidyLLVMModule + clangTidyModernizeModule clangTidyObjCModule clangTidyReadabilityModule clangTidyUtils Index: unittests/clang-tidy/ModernizerModuleTest.cpp =================================================================== --- /dev/null +++ unittests/clang-tidy/ModernizerModuleTest.cpp @@ -0,0 +1,64 @@ +#include "ClangTidyTest.h" +#include "modernize/UseDefaultMemberInitCheck.h" +#include "gtest/gtest.h" + +using namespace clang::tidy::modernize; + +namespace clang { +namespace tidy { +namespace test { + +TEST(UseDefaultMemberInitCheckTest, NoChanges) { + EXPECT_NO_CHANGES( + UseDefaultMemberInitCheck, + "struct S { S() = default; bool a_ = false; bool b_ = false; };"); + EXPECT_NO_CHANGES(UseDefaultMemberInitCheck, "struct S { S() : a_(true), " + "b_(true) {} bool a_{false}; " + "bool b_{false}; };"); +} + +TEST(UseDefaultMemberInitCheckTest, Basic) { + EXPECT_EQ("struct S { S() {} bool a_{false}; };", + runCheckOnCode( + "struct S { S() : a_(false) {} bool a_; };")); +} + +TEST(UseDefaultMemberInitCheckTest, SeveralInitializers) { + EXPECT_EQ( + "struct S { S() {} bool a_{false}; bool b_{true}; };", + runCheckOnCode( + "struct S { S() : a_(false), b_(true) {} bool a_; bool b_; };")); +} + +TEST(UseDefaultMemberInitCheckTest, ExceptSpec) { + EXPECT_EQ( + "struct S { S() noexcept(true) {} bool a_{false}; bool b_{true}; };", + runCheckOnCode( + "struct S { S() noexcept(true) : a_(false), b_(true) {} bool a_; " + "bool b_; };")); + EXPECT_EQ( + "#define NOEXCEPT_(X) noexcept(X)\n" + "struct S { S() NOEXCEPT_(true) {} bool a_{false}; bool b_{true}; };", + runCheckOnCode( + "#define NOEXCEPT_(X) noexcept(X)\n" + "struct S { S() NOEXCEPT_(true) : a_(false), b_(true) {} bool a_; " + "bool b_; };")); +} + +TEST(UseDefaultMemberInitCheckTest, OnExisting) { + EXPECT_EQ("struct S { S() {} bool a_{false}; bool b_{true}; };", + runCheckOnCode( + "struct S { S() : a_(false), b_(true) {} bool a_{false}; bool " + "b_{true}; };")); + EXPECT_EQ("struct S { S() : a_(true) {} bool a_{false}; bool b_{true}; };", + runCheckOnCode( + "struct S { S() : a_(true), b_(true) {} bool a_{false}; bool " + "b_{true}; };")); + EXPECT_EQ("struct S { S() : b_(false) {} bool a_{false}; bool b_{true}; };", + runCheckOnCode( + "struct S { S() : a_(false), b_(false) {} bool a_{false}; bool " + "b_{true}; };")); +} +} // namespace test +} // namespace tidy +} // namespace clang