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 <https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wextra>".
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 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
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
@@ -20,6 +20,10 @@
namespace tidy {
namespace readability {
+void RedundantMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IgnoreBaseInCopyConstructors", IgnoreBaseInCopyConstructors );
+}
+
void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
if (!getLangOpts().CPlusPlus)
return;
@@ -40,13 +44,19 @@
withInitializer(ignoringImplicit(Construct)),
unless(forField(hasType(isConstQualified()))),
unless(forField(hasParent(recordDecl(isUnion())))))
- .bind("init"))),
+ .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()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
