[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG980653621ef5: [clang-tidy] Give 
readability-redundant-member-init an option… (authored by mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D69145?vs=229934=230077#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  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
@@ -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 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() {}
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
@@ -6,7 +6,8 @@
 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 @@
   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 ‘Bar’ 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;
+  };
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,13 @@
   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
+  ` check.
+
+  The check  now supports the ``IgnoreBaseInCopyConstructors`` option to avoid
+  `"base class \‘Foo\’ should be explicitly initialized in the copy constructor"`
+  warnings or errors with ``gcc -Wextra`` or ``gcc -Werror=extra``.
+
 - The :doc:`readability-redundant-string-init
   ` check now supports a
   `StringNames` option enabling its application to custom string classes.
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Thank you, looks good to me.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

Thanks @lebedev.ri for taking the time to think this through and reply. All 
that makes sense, so I've changed the default to `0`.

In addition to adding it to the ReleaseNotes, once clang-tidy 10 is released 
I'll reply to a StackOverflow question about this error with a link to the new 
option. That should further increase the odds of anyone who searches on the 
`gcc` warning/error easily finding the clang-tidy option.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc updated this revision to Diff 229934.
poelmanc edited the summary of this revision.
poelmanc added a comment.

Switch default to `0`. Add Release Note with some detail to increase the 
chances of someone finding this with  an Internet search on the error message.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  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
@@ -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 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() {}
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
@@ -6,7 +6,8 @@
 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 @@
   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 ‘Bar’ 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;
+  };
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,13 @@
   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
+  ` check.
+
+  The check  now supports the ``IgnoreBaseInCopyConstructors`` option to avoid
+  `"base class \‘Foo\’ should be explicitly initialized in the copy constructor"`
+  warnings or errors with ``gcc -Wextra`` or ``gcc -Werror=extra``.
+
 - The :doc:`readability-redundant-string-init
   ` check now supports a
   `StringNames` option enabling its application to custom string classes.
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D69145#1750529 , @aaron.ballman 
wrote:

> In D69145#1748733 , @lebedev.ri 
> wrote:
>
> > In D69145#1748716 , @poelmanc 
> > wrote:
> >
> > > In D69145#1715611 , @mgehre 
> > > wrote:
> > >
> > > > Exactly due to the issue you are fixing here, we ended up disabling the 
> > > > complete check because we didn't want to live with the warnings it 
> > > > produced on -Wextra.
> > > >  Therefore, I'm actually strongly in favor to enable the option by 
> > > > default.
> > > >
> > > > When others see that clang-tidy fixits introduce warnings (with 
> > > > -Wextra) or even break their build (with -Werror), they might not look 
> > > > into check options, but just disable the check directly.
> > >
> > >
> > > Just pinging to see if anyone has any thoughts on moving forward with 
> > > this. Thanks in advance for any feedback!
> >
> >
> > If this would default to off i'd signoff.
>
>
> It sounds like you and @mgehre would like a different default for the new 
> option?


I suppose. It's not unlike "clang -Wall is not supposed to match gcc's -Wall" 
issue.
Everyone may expect different behavior. Defaulting to on (don't warn) isn't 
misguided,
there's merit in not changing the code if it would break some other compiler 
the code supports.

But the other way around is also true, what if that gcc's diagnostic is not 
enabled,
or gcc is just not used? One then would need to actually know that there is an 
option
that should be flipped. **Here** i would prefer to have to disable something
if it's unwanted rather than somehow finding out that something can be enabled..

As a more meaningful data point, a similar relaxation to another check was 
added in D70165 ,
and there the default is to still warn - so not following the precedent seems 
inconsistent to me.

> Personally, I don't have a strong opinion on the default, but



> I do think we should have the option so that users can control the behavior 
> so it doesn't conflict between clang-tidy and gcc.

Yes, i don't disagree that having *an ability* to control this is good.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69145#1748733 , @lebedev.ri wrote:

> In D69145#1748716 , @poelmanc wrote:
>
> > In D69145#1715611 , @mgehre wrote:
> >
> > > Exactly due to the issue you are fixing here, we ended up disabling the 
> > > complete check because we didn't want to live with the warnings it 
> > > produced on -Wextra.
> > >  Therefore, I'm actually strongly in favor to enable the option by 
> > > default.
> > >
> > > When others see that clang-tidy fixits introduce warnings (with -Wextra) 
> > > or even break their build (with -Werror), they might not look into check 
> > > options, but just disable the check directly.
> >
> >
> > Just pinging to see if anyone has any thoughts on moving forward with this. 
> > Thanks in advance for any feedback!
>
>
> If this would default to off i'd signoff.


It sounds like you and @mgehre would like a different default for the new 
option? Personally, I don't have a strong opinion on the default, but I do 
think we should have the option so that users can control the behavior so it 
doesn't conflict between clang-tidy and gcc.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a subscriber: mgehre.
poelmanc added a comment.

In D69145#1715611 , @mgehre wrote:

> Exactly due to the issue you are fixing here, we ended up disabling the 
> complete check because we didn't want to live with the warnings it produced 
> on -Wextra.
>  Therefore, I'm actually strongly in favor to enable the option by default.
>
> When others see that clang-tidy fixits introduce warnings (with -Wextra) or 
> even break their build (with -Werror), they might not look into check 
> options, but just disable the check directly.


Just pinging to see if anyone has any thoughts on moving forward with this. 
Thanks in advance for any feedback!


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D69145#1748716 , @poelmanc wrote:

> In D69145#1715611 , @mgehre wrote:
>
> > Exactly due to the issue you are fixing here, we ended up disabling the 
> > complete check because we didn't want to live with the warnings it produced 
> > on -Wextra.
> >  Therefore, I'm actually strongly in favor to enable the option by default.
> >
> > When others see that clang-tidy fixits introduce warnings (with -Wextra) or 
> > even break their build (with -Werror), they might not look into check 
> > options, but just disable the check directly.
>
>
> Just pinging to see if anyone has any thoughts on moving forward with this. 
> Thanks in advance for any feedback!


If this would default to off i'd signoff.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

In D69145#1725026 , @malcolm.parsons 
wrote:

> I like that this check warns about copy constructors that don't copy.
>  The warning can be suppressed with a NOLINT comment if not copying is 
> intentional.


Thanks for the comment! It's not clear to me how you suggest proceeding though.

I think it would be great to have a warning about "copy constructors that don't 
copy", which I would expect to warn regardless of whether the default 
initialization is explicit, e.g.

  MyClass( const MyClass& other) : BaseClass() {...}`

or implicit, e.g.

  MyClass( const MyClass& other) {...}

An intelligent "copy constructors that don't copy" warning might even examine 
the `{...}` to see what's being handled there.

Unfortunately currently we have two tools each of which warns in one case but 
not the other:

- `gcc -Wextra` warns only in the latter case, and can be fixed by changing 
each occurrence to the former.
- clang-tidy's current `readability-redundant-member-init` check warns only in 
the former case, and fixes it by changing each occurrence to the latter.
  - (Also clang-tidy's warning about `: BaseClass()` being redundant isn't a 
very clear warning about copy constructor that don't copy.)

This patch just offers an option to make peace between the two tools.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

I like that this check warns about copy constructors that don't copy.
The warning can be suppressed with a NOLINT comment if not copying is 
intentional.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc updated this revision to Diff 226847.
poelmanc added a comment.

Changed default to 1, updated help accordingly, and removed an extraneous set 
of braces around a one-line statement.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  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
@@ -1,4 +1,8 @@
 // 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 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() {}
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
@@ -6,7 +6,8 @@
 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 @@
   private:
 std::string s;
   };
+
+Options
+---
+
+.. option:: IgnoreBaseInCopyConstructors
+
+Default is ``1``.
+
+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 ‘Bar’ 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;
+  };
+
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -23,9 +23,15 @@
 class RedundantMemberInitCheck : public ClangTidyCheck {
 public:
   RedundantMemberInitCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreBaseInCopyConstructors(
+Options.get("IgnoreBaseInCopyConstructors", 1)) {}
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+  bool IgnoreBaseInCopyConstructors;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
===
--- 

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69145#1720254 , @poelmanc wrote:

> What do @malcolm.parsons, @alexfh, @hokein, @aaron.ballman, @lebedev.ri think 
> of @mgehre's suggestion to enable `IgnoreBaseInCopyConstructors` as the 
> default setting, so gcc users won't experience build errors and think 
> "clang-tidy broke my code!"
>
> I could go either way and appreciate any input.


I think GCC is being overly pedantic, but I don't see much harm in enabling the 
option by default.




Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp:60
+  if (IgnoreBaseInCopyConstructors && ConstructorDecl->isCopyConstructor() &&
+  Init->isBaseInitializer()) {
+return;

You should elide the braces here per our coding standard.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

What do @malcolm.parsons, @alexfh, @hokein, @aaron.ballman, @lebedev.ri think 
of @mgehre's suggestion to enable `IgnoreBaseInCopyConstructors` as the default 
setting, so gcc users won't experience build errors and think "clang-tidy broke 
my code!"

I could go either way and appreciate any input.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc updated this revision to Diff 225897.
poelmanc added a comment.

Rebase to latest master (tests moved into new "checkers" directory.)


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  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
@@ -1,4 +1,8 @@
 // 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 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() {}
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
@@ -6,7 +6,8 @@
 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 @@
   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 ‘Bar’ 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;
+  };
+
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -23,9 +23,15 @@
 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 ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+  bool IgnoreBaseInCopyConstructors;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
+++ 

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc marked 2 inline comments as done.
poelmanc added a comment.

Addressed these the other day but failed to check the "Done" boxes. Done!


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

In D69145#1715614 , @lebedev.ri wrote:

> So https://godbolt.org/z/qzjU-C
>  This feels like gcc being overly zealous, i'm not sure what it says with 
> that warning.


I'd agree, while copy constructors //usually// need to copy base class 
information as well, and forgetting to do so //may// indicate a programming 
error, this warning may be more appropriate as a linter suggestion than a 
compiler warning. (Usually you'd want `: BaseClass(other)`; `: BaseClass()` is 
really just silencing the warning so you can handle `BaseClass` in the body of 
the copy constructor.)

That said, the warning appears to have been introduced in gcc 4.0 

 in 2005  so every modern version of gcc will 
produce this warning. So in practical terms the concerns raised by @mgehre are 
why I feel this option is needed.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc updated this revision to Diff 225764.
poelmanc added a comment.

Addressed @mgehre's feedback:

- Ran clang-format
- Included gcc, -Wextra, -Werror=extra, and the full text of the warning/error 
message to aid in searchability, so people can find this option when they 
encounter the problem. Also added example code to the documentation.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
@@ -1,4 +1,8 @@
 // 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 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() {}
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
@@ -6,7 +6,8 @@
 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 @@
   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 ‘Bar’ 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;
+  };
+
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -23,9 +23,15 @@
 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 ) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+  bool IgnoreBaseInCopyConstructors;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

In D69145#1715611 , @mgehre wrote:

> Exactly due to the issue you are fixing here, we ended up disabling the 
> complete check because we didn't want to live with the warnings it produced 
> on -Wextra.
>  Therefore, I'm actually strongly in favor to enable the option by default.
>
> When others see that clang-tidy fixits introduce warnings (with -Wextra) or 
> even break their build (with -Werror), they might not look into check 
> options, but just disable the check directly.


Thank you for the feedback! I certainly like your suggestion to enable this by 
default but will await a collective conclusion before changing that.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

So https://godbolt.org/z/qzjU-C
This feels like gcc being overly zealous, i'm not sure what it says with that 
warning.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Exactly due to the issue you are fixing here, we ended up disabling the 
complete check because we didn't want to live with the warnings it produced on 
-Wextra.
Therefore, I'm actually strongly in favor to enable the option by default.

When others see that clang-tidy fixits introduce warnings (with -Wextra) or 
even break their build (with -Werror), they might not look into check options, 
but just disable the check directly.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp:55
   const auto *Construct = 
Result.Nodes.getNodeAs("construct");
+  const auto* ConstructorDecl = Result.Nodes.getNodeAs( 
"constructor" );
+

Extra space around string literal - did you run clang-format on your changes?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst:30
+within copy constructors. Some compilers issue warnings requiring copy
+constructors to explicitly initialize their base classes.
+

I'd might be helpful to mention `gcc` and `-Wextra` here so users can easily 
identify whether this case applies to them.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc created this revision.
poelmanc added reviewers: malcolm.parsons, alexfh, hokein, aaron.ballman.
poelmanc added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

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 false 
(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.

I blindly followed examples for adding a new option and would greatly 
appreciate any review/suggestions/feedback. Thank you.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
@@ -1,4 +1,8 @@
 // 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 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() {}
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
@@ -6,7 +6,8 @@
 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,14 @@
   private:
 std::string s;
   };
+
+Options
+---
+
+.. option:: IgnoreBaseInCopyConstructors
+
+When non-zero, the check will ignore unnecessary base class initializations
+within copy constructors. Some compilers issue warnings requiring copy
+constructors to explicitly initialize their base classes.
+
+Default is ``0``.  
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -23,9 +23,14 @@
 class RedundantMemberInitCheck : public ClangTidyCheck {
 public:
   RedundantMemberInitCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreBaseInCopyConstructors(Options.get("IgnoreBaseInCopyConstructors", 0))
+  {}
+  void