[clang-tidy] Give readability-redundant-member-init an option IgnoreBaseInCopyConstru...

Summary:
readability-redundant-member-init removes redundant / unnecessary member and base class initialization. Unfortunately for the specific case of a copy constructor's initialization of a base class, gcc at strict warning levels warns if "base class is not initialized in the copy constructor of a derived class".

This patch adds an option `IgnoreBaseInCopyConstructors` defaulting to 0 (thus maintaining current behavior by default) to skip the specific case of removal of redundant base class initialization in the copy constructor. Enabling this option enables the resulting code to continue to compile successfully under `gcc -Werror=extra`. New test cases `WithCopyConstructor1` and `WithCopyConstructor2` in clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp show that it removes redundant members even from copy constructors.

Reviewers: malcolm.parsons, alexfh, hokein, aaron.ballman, lebedev.ri

Patch by: poelmanc

Subscribers: mgehre, lebedev.ri, cfe-commits

Tags: #clang, #clang-tools-extra

Differential revision: https://reviews.llvm.org/D69145

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
index d38d0a233e4b3421c25a33d5106bb331eb46addd..cf48bbdc621049cf6994e376336e26a6bb660b08 100644 (file)
--- a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
@@ -20,6 +20,11 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
+void RedundantMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreBaseInCopyConstructors",
+                IgnoreBaseInCopyConstructors);
+}
+
 void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
@@ -36,17 +41,24 @@ void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
           ofClass(unless(
               anyOf(isUnion(), ast_matchers::isTemplateInstantiation()))),
           forEachConstructorInitializer(
-              cxxCtorInitializer(isWritten(),
-                                 withInitializer(ignoringImplicit(Construct)),
-                                 unless(forField(hasType(isConstQualified()))),
-                                 unless(forField(hasParent(recordDecl(isUnion())))))
-                  .bind("init"))),
+              cxxCtorInitializer(
+                  isWritten(), withInitializer(ignoringImplicit(Construct)),
+                  unless(forField(hasType(isConstQualified()))),
+                  unless(forField(hasParent(recordDecl(isUnion())))))
+                  .bind("init")))
+          .bind("constructor"),
       this);
 }
 
 void RedundantMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Init = Result.Nodes.getNodeAs<CXXCtorInitializer>("init");
   const auto *Construct = Result.Nodes.getNodeAs<CXXConstructExpr>("construct");
+  const auto *ConstructorDecl =
+      Result.Nodes.getNodeAs<CXXConstructorDecl>("constructor");
+
+  if (IgnoreBaseInCopyConstructors && ConstructorDecl->isCopyConstructor() &&
+      Init->isBaseInitializer())
+    return;
 
   if (Construct->getNumArgs() == 0 ||
       Construct->getArg(0)->isDefaultArgument()) {

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
index 73ced118b1559761c5f23aef2cdcc152982de31a..b8e11a6ceee9aed88b1b4c896f8b103958976e26 100644 (file)
--- a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -23,9 +23,15 @@ namespace readability {
 class RedundantMemberInitCheck : public ClangTidyCheck {
 public:
   RedundantMemberInitCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        IgnoreBaseInCopyConstructors(
+            Options.get("IgnoreBaseInCopyConstructors", 0)) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  bool IgnoreBaseInCopyConstructors;
 };
 
 } // namespace readability

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b9feee29d137a20ea9386131358d7104e347b97d..bee80210527bbecbbe00deb52e71ba635ab6f80b 100644 (file)
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,13 @@ Improvements to clang-tidy
   The check now supports the ``AllowOverrideAndFinal`` option to eliminate
   conflicts with ``gcc -Wsuggest-override`` or ``gcc -Werror=suggest-override``.
 
+- Improved :doc:`readability-redundant-member-init
+  <clang-tidy/checks/readability-redundant-member-init>` check.
+
+  The check  now supports the ``IgnoreBaseInCopyConstructors`` option to avoid
+  `"base class \\91Foo\\92 should be explicitly initialized in the copy constructor"`
+  warnings or errors with ``gcc -Wextra`` or ``gcc -Werror=extra``.
+
 - The :doc:`readability-redundant-string-init
   <clang-tidy/checks/readability-redundant-string-init>` check now supports a
   `StringNames` option enabling its application to custom string classes.

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 a640e16e3c099fb82a5bf34958684afa227bd8dd..116de1f3dd36ef6d852d891bf798732def565d2b 100644 (file)
--- 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
@@ -6,7 +6,8 @@ readability-redundant-member-init
 Finds member initializations that are unnecessary because the same default
 constructor would be called if they were not present.
 
-Example:
+Example
+-------
 
 .. code-block:: c++
 
@@ -18,3 +19,27 @@ Example:
   private:
     std::string s;
   };
+
+Options
+-------
+
+.. option:: IgnoreBaseInCopyConstructors
+
+    Default is ``0``.
+
+    When non-zero, the check will ignore unnecessary base class initializations
+    within copy constructors, since some compilers issue warnings/errors when
+    base classes are not explicitly intialized in copy constructors. For example,
+    ``gcc`` with ``-Wextra`` or ``-Werror=extra`` issues warning or error
+    ``base class \91Bar\92 should be explicitly initialized in the copy constructor``
+    if ``Bar()`` were removed in the following example:
+
+.. code-block:: c++
+
+  // Explicitly initializing member s and base class Bar is unnecessary.
+  struct Foo : public Bar {
+    // Remove s() below. If IgnoreBaseInCopyConstructors!=0, keep Bar().
+    Foo(const Foo& foo) : Bar(), s() {}
+    std::string s;
+  };
+

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 90b52fd1a48c996b439837da4d357b0e9b69770e..0f8c028aabb746c8f3c0e82ae0dc1bd2632e1d71 100644 (file)
--- 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
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy %s readability-redundant-member-init %t
+// RUN: %check_clang_tidy %s readability-redundant-member-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [{key: readability-redundant-member-init.IgnoreBaseInCopyConstructors, \
+// RUN:               value: 1}] \
+// RUN:             }"
 
 struct S {
   S() = default;
@@ -116,6 +120,35 @@ struct F9 {
   };
 };
 
+// struct whose inline copy constructor default-initializes its base class
+struct WithCopyConstructor1 : public T {
+  WithCopyConstructor1(const WithCopyConstructor1& other) : T(),
+    f(),
+    g()
+  {}
+  S f, g;
+};
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'f' is redundant
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'g' is redundant
+// CHECK-FIXES: WithCopyConstructor1(const WithCopyConstructor1& other) : T()
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: {}
+
+// struct whose copy constructor default-initializes its base class
+struct WithCopyConstructor2 : public T {
+  WithCopyConstructor2(const WithCopyConstructor2& other);
+  S a;
+};
+WithCopyConstructor2::WithCopyConstructor2(const WithCopyConstructor2& other)
+  : T(), a()
+{}
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: initializer for member 'a' is redundant
+// CHECK-FIXES: {{^}}  : T() {{$}}
+// CHECK-NEXT: {}
+
 // Initializer not written
 struct NF1 {
   NF1() {}