[clang-tidy] Add check 'readability-redundant-member-init'

[malcolm.parsons]

Summary

The check emits a warning if a member-initializer calls the member's default constructor with no arguments.

[Eugene.Zelenko]

I think will be good idea to add cases when member is initialized in declaration and constructor, with same and different values.

docs/clang-tidy/checks/readability-redundant-member-init.rst

4 ↗

(On Diff #70684)

This line length should be same as header above.

13 ↗

(On Diff #70684)

Please run Clang-format over example. Will be good idea to add empty line before private.

test/clang-tidy/readability-redundant-member-init.cpp

33 ↗

(On Diff #70684)

Please remove extra empty-line.

[Prazek]

clang-tidy/readability/RedundantMemberInitCheck.cpp

34 ↗

(On Diff #70684)

Arguments (upper case)

[Eugene.Zelenko]

clang-tidy/readability/RedundantMemberInitCheck.cpp

36 ↗

(On Diff #70684)

begin() and end() are not used extensively. Why not to use std::?

[sbenza]

clang-tidy/readability/RedundantMemberInitCheck.cpp

33 ↗

(On Diff #70684)

These construct expressions might actually have side effects that you would not get if you omit them.
For example, aggregates will be value initialized. If you remove it you change the behavior.

The tests should include defaulted vs non-defaulted default constructor, user-defined/user-provided/implicit default constructor, aggregates with and without trivially constructible members, etc.

34 ↗

(On Diff #70684)

Arguments variable name. (should start with upper case)

36 ↗

(On Diff #70684)

There's no need for these using declarations.
Just do arguments.begin()/arguments.end().
They are using in generic code. This is not generic code.

39 ↗

(On Diff #70684)

You want std::none_of instead of std::find_if.
(or use std::any_of and don't negate the expression in the lambda)

docs/clang-tidy/checks/readability-redundant-member-init.rst

6 ↗

(On Diff #70684)

Explain when they are unnecessary.

[aaron.ballman]

clang-tidy/readability/RedundantMemberInitCheck.cpp

34 ↗

(On Diff #70684)

Please do not use auto here, since the type is not spelled out in the initializer.

clang-tidy/readability/RedundantMemberInitCheck.h

19 ↗

(On Diff #70684)

What makes one unnecessary? Also, missing a full stop at the end of the sentence.

[malcolm.parsons]

How do I add FixIt hints?
They should be simple removals, but how do I decide whether to remove the following comma, preceding comma or preceding colon?

clang-tidy/readability/RedundantMemberInitCheck.cpp

33 ↗

(On Diff #70684)

Yes, more tests needed.

39 ↗

(On Diff #70684)

Maybe I only need to check the first argument - if that's defaulted then they all are.

docs/clang-tidy/checks/readability-redundant-member-init.rst

6 ↗

(On Diff #70684)

"Finds member initializations that are unnecessary because the same default constructor would be called if they were not present"

[malcolm.parsons]

Improve comments
Improve doc
Add tests
Simplify implementation

[malcolm.parsons]

clang-tidy/readability/RedundantMemberInitCheck.cpp

34 ↗

(On Diff #70797)

Other forms of initialization are not CXXConstructExprs.

[alexfh]

In D24339#537488, @malcolm.parsons wrote:

How do I add FixIt hints?
They should be simple removals, but how do I decide whether to remove the following comma, preceding comma or preceding colon?

Just remove the entry and leave the comma and the colon alone. Clang-tidy needs to learn to clean up the code after such replacements using clang-format's cleanupAroundReplacements. Filed https://llvm.org/bugs/show_bug.cgi?id=30379

[malcolm.parsons]

Handle unions and templated classes.
Add FixItHints (depends on D24572).

[malcolm.parsons]

Handle delegating and base class constructors

[malcolm.parsons]

Don't remove init of const members.
Do remove calls to constructors that introduce cleanups.

[malcolm.parsons]

Don't warn for trivially default constructable members

[aaron.ballman]

test/clang-tidy/readability-redundant-member-init.cpp

162 ↗

(On Diff #73260)

Missing a test for union member initializers. Also, a test that multiple inheritance doesn't cause a fixit malfunction would be nice.

[malcolm.parsons]

test/clang-tidy/readability-redundant-member-init.cpp

162 ↗

(On Diff #73260)

Note that all the fixits malfunction until D24572 lands.

[malcolm.parsons]

Add test for initializing a union member.
Add test for multiple inheritance.

[aaron.ballman]

LGTM!

[malcolm.parsons]

Use ignoringImplicit

clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantControlFlowCheck.cpp
+  RedundantMemberInitCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringInitCheck.cpp
   SimplifyBooleanExprCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "FunctionSizeCheck.h"
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
+#include "RedundantMemberInitCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 
 namespace clang {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<IdentifierNamingCheck>(
         "readability-identifier-naming");
     CheckFactories.registerCheck<ImplicitBoolCastCheck>(
         "readability-implicit-bool-cast");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
         "readability-misplaced-array-index");
+    CheckFactories.registerCheck<RedundantMemberInitCheck>(
+        "readability-redundant-member-init");
     CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
         "readability-static-definition-in-anonymous-namespace");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<NonConstParameterCheck>(
         "readability-non-const-parameter");
     CheckFactories.registerCheck<RedundantControlFlowCheck>(
         "readability-redundant-control-flow");

clang-tools-extra/trunk/clang-tidy/readability/RedundantMemberInitCheck.h

+//===--- RedundantMemberInitCheck.h - clang-tidy----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_MEMBER_INIT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_MEMBER_INIT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds member initializations that are unnecessary because the same default
+/// constructor would be called if they were not present.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-member-init.html
+class RedundantMemberInitCheck : public ClangTidyCheck {
+public:
+  RedundantMemberInitCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_MEMBER_INIT_H

clang-tools-extra/trunk/clang-tidy/readability/RedundantMemberInitCheck.cpp

+//===--- RedundantMemberInitCheck.cpp - clang-tidy-------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "RedundantMemberInitCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "../utils/Matchers.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+using namespace clang::tidy::matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
+  auto Construct =
+      cxxConstructExpr(
+          hasDeclaration(cxxConstructorDecl(hasParent(
+              cxxRecordDecl(unless(isTriviallyDefaultConstructible()))))))
+          .bind("construct");
+
+  Finder->addMatcher(
+      cxxConstructorDecl(
+          unless(isDelegatingConstructor()),
+          ofClass(unless(
+              anyOf(isUnion(), ast_matchers::isTemplateInstantiation()))),
+          forEachConstructorInitializer(
+              cxxCtorInitializer(isWritten(),
+                                 withInitializer(ignoringImplicit(Construct)),
+                                 unless(forField(hasType(isConstQualified()))))
+                  .bind("init"))),
+      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 (Construct->getNumArgs() == 0 ||
+      Construct->getArg(0)->isDefaultArgument()) {
+    if (Init->isAnyMemberInitializer()) {
+      diag(Init->getSourceLocation(), "initializer for member %0 is redundant")
+          << Init->getMember()
+          << FixItHint::CreateRemoval(Init->getSourceRange());
+    } else {
+      diag(Init->getSourceLocation(),
+           "initializer for base class %0 is redundant")
+          << Init->getTypeSourceInfo()->getType()
+          << FixItHint::CreateRemoval(Init->getSourceRange());
+    }
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/docs/ReleaseNotes.rst

- New `readability-misplaced-array-index
   Warns when there is array index before the [] instead of inside it.
 
 - New `readability-non-const-parameter
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-non-const-parameter.html>`_ check
 
   Flags function parameters of a pointer type that could be changed to point to
   a constant type instead.
 
+- New `readability-redundant-member-init
+  <http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-member-init.html>`_ check
+
+  Flags member initializations that are unnecessary because the same default
+  constructor would be called if they were not present.
+
 Fixed bugs:
 
 - `modernize-make-unique
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-make-unique.html>`_
   and `modernize-make-shared
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-make-shared.html>`_
   Calling ``make_{unique|shared}`` from within a member function of a type
   with a private or protected constructor would be ill-formed.

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    readability-function-size
    readability-identifier-naming
    readability-implicit-bool-cast
    readability-inconsistent-declaration-parameter-name
    readability-misplaced-array-index
    readability-named-parameter
    readability-non-const-parameter
    readability-redundant-control-flow
+   readability-redundant-member-init
    readability-redundant-smartptr-get
    readability-redundant-string-cstr
    readability-redundant-string-init
    readability-simplify-boolean-expr
    readability-static-definition-in-anonymous-namespace
    readability-uniqueptr-delete-release

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-redundant-member-init.rst

+.. title:: clang-tidy - readability-redundant-member-init
+
+readability-redundant-member-init
+=================================
+
+Finds member initializations that are unnecessary because the same default
+constructor would be called if they were not present.
+
+Example:
+
+.. code-block:: c++
+
+  // Explicitly initializing the member s is unnecessary.
+  class Foo {
+  public:
+    Foo() : s() {}
+
+  private:
+    std::string s;
+  };

clang-tools-extra/trunk/test/clang-tidy/readability-redundant-member-init.cpp

+// RUN: %check_clang_tidy %s readability-redundant-member-init %t
+
+struct S {
+  S() = default;
+  S(int i) : i(i) {}
+  int i = 1;
+};
+
+struct T {
+  T(int i = 1) : i(i) {}
+  int i;
+};
+
+struct U {
+  int i;
+};
+
+union V {
+  int i;
+  double f;
+};
+
+// Initializer calls default constructor
+struct F1 {
+  F1() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for member 'f' is redundant
+  // CHECK-FIXES: F1()  {}
+  S f;
+};
+
+// Initializer calls default constructor with default argument
+struct F2 {
+  F2() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for member 'f' is redundant
+  // CHECK-FIXES: F2()  {}
+  T f;
+};
+
+// Multiple redundant initializers for same constructor
+struct F3 {
+  F3() : f(), g(1), h() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for member 'f' is redundant
+  // CHECK-MESSAGES: :[[@LINE-2]]:21: warning: initializer for member 'h' is redundant
+  // CHECK-FIXES: F3() :  g(1) {}
+  S f, g, h;
+};
+
+// Templated class independent type
+template <class V>
+struct F4 {
+  F4() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for member 'f' is redundant
+  // CHECK-FIXES: F4()  {}
+  S f;
+};
+F4<int> f4i;
+F4<S> f4s;
+
+// Base class
+struct F5 : S {
+  F5() : S() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for base class 'S' is redundant
+  // CHECK-FIXES: F5()  {}
+};
+
+// Constructor call requires cleanup
+struct Cleanup {
+  ~Cleanup() {}
+};
+
+struct UsesCleanup {
+  UsesCleanup(const Cleanup &c = Cleanup()) {}
+};
+
+struct F6 {
+  F6() : uc() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for member 'uc' is redundant
+  // CHECK-FIXES: F6()  {}
+  UsesCleanup uc;
+};
+
+// Multiple inheritance
+struct F7 : S, T {
+  F7() : S(), T() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for base class 'S' is redundant
+  // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: initializer for base class 'T' is redundant
+  // CHECK-FIXES: F7()  {}
+};
+
+// Initializer not written
+struct NF1 {
+  NF1() {}
+  S f;
+};
+
+// Initializer doesn't call default constructor
+struct NF2 {
+  NF2() : f(1) {}
+  S f;
+};
+
+// Initializer calls default constructor without using default argument
+struct NF3 {
+  NF3() : f(1) {}
+  T f;
+};
+
+// Initializer calls default constructor without using default argument
+struct NF4 {
+  NF4() : f(2) {}
+  T f;
+};
+
+// Initializer is zero-initialization
+struct NF5 {
+  NF5() : i() {}
+  int i;
+};
+
+// Initializer is direct-initialization
+struct NF6 {
+  NF6() : i(1) {}
+  int i;
+};
+
+// Initializer is aggregate initialization of struct
+struct NF7 {
+  NF7() : f{} {}
+  U f;
+};
+
+// Initializer is zero-initialization of struct
+struct NF7b {
+  NF7b() : f() {}
+  U f;
+};
+
+// Initializer is aggregate initialization of array
+struct NF8 {
+  NF8() : f{} {}
+  int f[2];
+};
+
+struct NF9 {
+  NF9() : f{} {}
+  S f[2];
+};
+
+// Initializing member of union
+union NF10 {
+  NF10() : s() {}
+  int i;
+  S s;
+};
+
+// Templated class dependent type
+template <class V>
+struct NF11 {
+  NF11() : f() {}
+  V f;
+};
+NF11<int> nf11i;
+NF11<S> nf11s;
+
+// Delegating constructor
+class NF12 {
+  NF12() = default;
+  NF12(int) : NF12() {}
+};
+
+// Const member
+struct NF13 {
+  NF13() : f() {}
+  const S f;
+};
+
+// Union member
+struct NF14 {
+  NF14() : f() {}
+  V f;
+};