diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "RedundantMemberInitCheck.h" +#include "../utils/LexerUtils.h" #include "../utils/Matchers.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -18,33 +19,64 @@ namespace clang::tidy::readability { +static SourceRange +getFullInitRangeInclWhitespaces(SourceRange Range, const SourceManager &SM, + const LangOptions &LangOpts) { + const Token PrevToken = + utils::lexer::getPreviousToken(Range.getBegin(), SM, LangOpts, false); + if (PrevToken.is(tok::unknown)) + return Range; + + if (PrevToken.isNot(tok::equal)) + return {PrevToken.getEndLoc(), Range.getEnd()}; + + return getFullInitRangeInclWhitespaces( + {PrevToken.getLocation(), Range.getEnd()}, SM, LangOpts); +} + void RedundantMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreBaseInCopyConstructors", IgnoreBaseInCopyConstructors); } void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) { + auto ConstructorMatcher = + cxxConstructExpr(argumentCountIs(0), + hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl( + unless(isTriviallyDefaultConstructible())))))) + .bind("construct"); + Finder->addMatcher( cxxConstructorDecl( unless(isDelegatingConstructor()), ofClass(unless(isUnion())), forEachConstructorInitializer( - cxxCtorInitializer( - withInitializer( - cxxConstructExpr( - hasDeclaration( - cxxConstructorDecl(ofClass(cxxRecordDecl( - unless(isTriviallyDefaultConstructible())))))) - .bind("construct")), - unless(forField(hasType(isConstQualified()))), - unless(forField(hasParent(recordDecl(isUnion()))))) + cxxCtorInitializer(withInitializer(ConstructorMatcher), + unless(forField(fieldDecl( + anyOf(hasType(isConstQualified()), + hasParent(recordDecl(isUnion()))))))) .bind("init"))) .bind("constructor"), this); + + Finder->addMatcher(fieldDecl(hasInClassInitializer(ConstructorMatcher), + unless(hasParent(recordDecl(isUnion())))) + .bind("field"), + this); } void RedundantMemberInitCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Init = Result.Nodes.getNodeAs("init"); const auto *Construct = Result.Nodes.getNodeAs("construct"); + + if (const auto *Field = Result.Nodes.getNodeAs("field")) { + const Expr *Init = Field->getInClassInitializer(); + diag(Construct->getExprLoc(), "initializer for member %0 is redundant") + << Field + << FixItHint::CreateRemoval(getFullInitRangeInclWhitespaces( + Init->getSourceRange(), *Result.SourceManager, getLangOpts())); + return; + } + + const auto *Init = Result.Nodes.getNodeAs("init"); const auto *ConstructorDecl = Result.Nodes.getNodeAs("constructor"); @@ -52,18 +84,15 @@ Init->isBaseInitializer()) return; - if (Construct->getNumArgs() == 0 || - Construct->getArg(0)->isDefaultArgument()) { - if (Init->isAnyMemberInitializer()) { - diag(Init->getSourceLocation(), "initializer for member %0 is redundant") - << Init->getAnyMember() - << FixItHint::CreateRemoval(Init->getSourceRange()); - } else { - diag(Init->getSourceLocation(), - "initializer for base class %0 is redundant") - << Construct->getType() - << FixItHint::CreateRemoval(Init->getSourceRange()); - } + if (Init->isAnyMemberInitializer()) { + diag(Init->getSourceLocation(), "initializer for member %0 is redundant") + << Init->getAnyMember() + << FixItHint::CreateRemoval(Init->getSourceRange()); + } else { + diag(Init->getSourceLocation(), + "initializer for base class %0 is redundant") + << Construct->getType() + << FixItHint::CreateRemoval(Init->getSourceRange()); } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -192,6 +192,10 @@ ` check to emit proper warnings when a type forward declaration precedes its definition. +- Improved :doc:`readability-redundant-member-init + ` check to support + in-class initializers. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst --- a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst @@ -11,13 +11,14 @@ .. code-block:: c++ - // Explicitly initializing the member s is unnecessary. + // Explicitly initializing the member s and v is unnecessary. class Foo { public: Foo() : s() {} private: std::string s; + std::vector v {}; }; Options diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp @@ -250,3 +250,55 @@ S s2; }; }; + +// Direct in-class initialization with default constructor +struct D1 { + S f1 {}; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: initializer for member 'f1' is redundant + // CHECK-FIXES: S f1; +}; + +// Direct in-class initialization with constructor with default argument +struct D2 { + T f2 {}; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: initializer for member 'f2' is redundant + // CHECK-FIXES: T f2; +}; + +// Direct in-class initialization with default constructor (assign) +struct D3 { + S f3 = {}; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: initializer for member 'f3' is redundant + // CHECK-FIXES: S f3; +}; + +// Direct in-class initialization with constructor with default argument (assign) +struct D4 { + T f4 = {}; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: initializer for member 'f4' is redundant + // CHECK-FIXES: T f4; +}; + +// Templated class independent type +template +struct D5 { + S f5 /*comment*/ = S(); + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: initializer for member 'f5' is redundant + // CHECK-FIXES: S f5 /*comment*/; +}; +D5 d5i; +D5 d5s; + +struct D6 { + UsesCleanup uc2{}; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: initializer for member 'uc2' is redundant + // CHECK-FIXES: UsesCleanup uc2; +}; + +template +struct D7 { + V f7; +}; + +D7 d7i; +D7 d7s;