Skip to content

[clang-tidy] Fix handling of members in readability-redundant-member-init #93217

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

Compare class type instead of just assuming
that called constructor belong to same class.

Fixes #91605

@llvmbot
Copy link
Member

llvmbot commented May 23, 2024

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Compare class type instead of just assuming
that called constructor belong to same class.

Fixes #91605


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp (+18-8)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp (+16)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
index 015347ee9294c..601ff44cdd10a 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
@@ -41,25 +41,35 @@ void RedundantMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 
 void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
   auto ConstructorMatcher =
-      cxxConstructExpr(argumentCountIs(0),
-                       hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl(
-                           unless(isTriviallyDefaultConstructible()))))))
+      cxxConstructExpr(
+          argumentCountIs(0),
+          hasDeclaration(cxxConstructorDecl(
+              ofClass(cxxRecordDecl(unless(isTriviallyDefaultConstructible()))
+                          .bind("class")))))
           .bind("construct");
 
+  auto HasUnionAsParent = hasParent(recordDecl(isUnion()));
+
+  auto HasTypeEqualToConstructorClass = hasType(qualType(
+      hasCanonicalType(qualType(hasDeclaration(equalsBoundNode("class"))))));
+
   Finder->addMatcher(
       cxxConstructorDecl(
           unless(isDelegatingConstructor()), ofClass(unless(isUnion())),
           forEachConstructorInitializer(
-              cxxCtorInitializer(withInitializer(ConstructorMatcher),
-                                 unless(forField(fieldDecl(
-                                     anyOf(hasType(isConstQualified()),
-                                           hasParent(recordDecl(isUnion())))))))
+              cxxCtorInitializer(
+                  withInitializer(ConstructorMatcher),
+                  anyOf(isBaseInitializer(),
+                        forField(fieldDecl(unless(hasType(isConstQualified())),
+                                           unless(HasUnionAsParent),
+                                           HasTypeEqualToConstructorClass))))
                   .bind("init")))
           .bind("constructor"),
       this);
 
   Finder->addMatcher(fieldDecl(hasInClassInitializer(ConstructorMatcher),
-                               unless(hasParent(recordDecl(isUnion()))))
+                               HasTypeEqualToConstructorClass,
+                               unless(HasUnionAsParent))
                          .bind("field"),
                      this);
 }
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 741abc0a199a7..8f3602ea5da39 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -387,6 +387,11 @@ Changes in existing checks
   <clang-tidy/checks/readability/redundant-inline-specifier>` check to properly
   emit warnings for static data member with an in-class initializer.
 
+- Improved :doc:`readability-redundant-member-init
+  <clang-tidy/checks/readability/redundant-member-init>` check to avoid
+  false-positives when type of the member does not match type of the
+  initializer.
+
 - Improved :doc:`readability-static-accessed-through-instance
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   support calls to overloaded operators as base expression and provide fixes to
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 17b2714abca07..6f18a6043be93 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
@@ -302,3 +302,19 @@ struct D7 {
 
 D7<int> d7i;
 D7<S> d7s;
+
+struct SS {
+  SS() = default;
+  SS(S s) : s(s) {}
+
+  S s;
+};
+
+struct D8 {
+  SS ss = S();
+};
+
+struct D9 {
+  D9() : ss(S()) {}
+  SS ss;
+};

Copy link
Contributor

@5chmidti 5chmidti left a comment

Choose a reason for hiding this comment

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

LGTM, with a release note nit.

PiotrZSL added 2 commits May 28, 2024 20:20
…init

Compare class type instead of just assuming
that called constructor belong to same class.

Fixes llvm#91605
@PiotrZSL PiotrZSL force-pushed the 91605-clang-tidy-readability-redundant-member-init-warning-when-non-redudant branch from 6402bdd to fc04fef Compare May 28, 2024 20:24
@PiotrZSL PiotrZSL merged commit 461dcd4 into llvm:main Jun 5, 2024
8 checks passed
@PiotrZSL PiotrZSL deleted the 91605-clang-tidy-readability-redundant-member-init-warning-when-non-redudant branch June 5, 2024 18:24
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] readability-redundant-member-init warning when non-redudant
3 participants