[clang-tools-extra] [clang-tidy] Add support for in-class initializers in readability-redundant-member-init (PR #77206)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 13 23:49:34 PST 2024


https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/77206

>From 66e4026ff568fbd805ddd777596102381e4e0066 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Sat, 6 Jan 2024 18:04:57 +0000
Subject: [PATCH 1/2] [clang-tidy] Add support for in-class initializers in
 readability-redundant-member-init

Support detecting redundant in-class initializers.
Moved from https://reviews.llvm.org/D157262

Fixes: #62525
---
 .../readability/RedundantMemberInitCheck.cpp  | 73 +++++++++++++------
 clang-tools-extra/docs/ReleaseNotes.rst       |  6 +-
 .../readability/redundant-member-init.rst     |  3 +-
 .../readability/redundant-member-init.cpp     | 52 +++++++++++++
 4 files changed, 110 insertions(+), 24 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
index b5d407773bb732..015347ee9294ce 100644
--- 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 @@ 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");
 
@@ -52,18 +84,15 @@ void RedundantMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
       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
index 1bd5a72126c10b..8ec3dcadc6ebe7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,7 +119,7 @@ Improvements to clang-tidy
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
-- Improved :program:`clang-tidy-diff.py` script. 
+- Improved :program:`clang-tidy-diff.py` script.
     * Return exit code `1` if any :program:`clang-tidy` subprocess exits with
       a non-zero code or if exporting fixes fails.
 
@@ -487,6 +487,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 support
+  in-class initializers.
+
 - Improved :doc:`readability-simplify-subscript-expr
   <clang-tidy/checks/readability/simplify-subscript-expr>` check by extending
   the default value of the `Types` option to include ``std::span``.
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
index b2f86c4429cc51..c26aaf73b16f85 100644
--- 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 @@ 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
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
index d8ca406581a385..17b2714abca07b 100644
--- 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 @@ 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;

>From 99dd7074a3ac27eedc5947febdd43d8c38ee4159 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Sun, 14 Jan 2024 07:49:22 +0000
Subject: [PATCH 2/2] Release notes update

---
 clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8ec3dcadc6ebe7..acd598f6c48065 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -488,8 +488,8 @@ Changes in existing checks
   false-positives in initializer list of record.
 
 - Improved :doc:`readability-redundant-member-init
-  <clang-tidy/checks/readability/redundant-member-init>` check to support
-  in-class initializers.
+  <clang-tidy/checks/readability/redundant-member-init>` check to now also
+  detect redundant in-class initializers.
 
 - Improved :doc:`readability-simplify-subscript-expr
   <clang-tidy/checks/readability/simplify-subscript-expr>` check by extending



More information about the cfe-commits mailing list