PiotrZSL created this revision.
PiotrZSL added reviewers: carlosgalvezp, njames93.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Support detecting redundant in-class initializers.
Fixes: #62525
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157262
Files:
clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp
+++ 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 <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;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst
+++ 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<int> v {};
};
Options
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,10 @@
<clang-tidy/checks/readability/identifier-naming>` check to emit proper
warnings when a type forward declaration precedes its definition.
+- Improved :doc:`readability-redundant-member-init
+ <clang-tidy/checks/readability/redundant-member-init>` check to support
+ in-class initializers.
+
Removed checks
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
+++ 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<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");
@@ -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());
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
