Skip to content

[clang-tidy] Add support for in-class initializers in readability-redundant-member-init #77206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -18,52 +19,80 @@ using namespace clang::tidy::matchers;

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<CXXCtorInitializer>("init");
const auto *Construct = Result.Nodes.getNodeAs<CXXConstructExpr>("construct");

if (const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("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<CXXCtorInitializer>("init");
const auto *ConstructorDecl =
Result.Nodes.getNodeAs<CXXConstructorDecl>("constructor");

if (IgnoreBaseInCopyConstructors && ConstructorDecl->isCopyConstructor() &&
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());
}
}

Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,10 @@ Changes in existing checks
<clang-tidy/checks/readability/non-const-parameter>` check to ignore
false-positives in initializer list of record.

- Improved :doc:`readability-redundant-member-init
<clang-tidy/checks/readability/redundant-member-init>` check to now also
detect redundant in-class initializers.

- Improved :doc:`readability-simplify-boolean-expr
<clang-tidy/checks/readability/simplify-boolean-expr>` check by adding the
new option `IgnoreMacros` that allows to ignore boolean expressions originating
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ Example

.. 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<int> v {};
};

Options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,3 +250,55 @@ struct NF15 {
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 <class V>
struct D5 {
S f5 /*comment*/ = S();
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: initializer for member 'f5' is redundant
// CHECK-FIXES: S f5 /*comment*/;
};
D5<int> d5i;
D5<S> d5s;

struct D6 {
UsesCleanup uc2{};
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: initializer for member 'uc2' is redundant
// CHECK-FIXES: UsesCleanup uc2;
};

template<typename V>
struct D7 {
V f7;
};

D7<int> d7i;
D7<S> d7s;