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

Conversation

PiotrZSL
Copy link
Member

@PiotrZSL PiotrZSL commented Jan 6, 2024

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

Fixes: #62525

…undant-member-init

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

Fixes: llvm#62525
@llvmbot
Copy link
Member

llvmbot commented Jan 6, 2024

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

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

Fixes: #62525


Full diff: https://github.com/llvm/llvm-project/pull/77206.diff

4 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp (+51-22)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5-1)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst (+2-1)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp (+52)
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;

@PiotrZSL PiotrZSL self-assigned this Jan 7, 2024
Copy link
Member

@phyBrackets phyBrackets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank You @PiotrZSL . Patch looks good to me. Just a couple of nit picks if it's okay for you.

@PiotrZSL PiotrZSL merged commit 59e79f0 into llvm:main Jan 14, 2024
@PiotrZSL PiotrZSL deleted the 62525-clang-tidy-extend-readability-redundant-member-init-to-non-static-data-member-initializers branch January 14, 2024 14:48
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…undant-member-init (llvm#77206)

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

Fixes: llvm#62525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-tidy] extend readability-redundant-member-init to non-static data member initializers
3 participants