[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

This line length should be same as header above.

13

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

Please remove extra empty-line.

[Prazek]

clang-tidy/readability/RedundantMemberInitCheck.cpp

34

Arguments (upper case)

[Eugene.Zelenko]

clang-tidy/readability/RedundantMemberInitCheck.cpp

36

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

[sbenza]

clang-tidy/readability/RedundantMemberInitCheck.cpp

33

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

Arguments variable name. (should start with upper case)

36

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

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

Explain when they are unnecessary.

[aaron.ballman]

clang-tidy/readability/RedundantMemberInitCheck.cpp

34

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

clang-tidy/readability/RedundantMemberInitCheck.h

19

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

Yes, more tests needed.

39

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

"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

35

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

163

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

163

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-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   IdentifierNamingCheck.cpp
   ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.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-tidy/readability/ReadabilityTidyModule.cpp

 #include "ElseAfterReturnCheck.h"
 #include "FunctionSizeCheck.h"
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.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<FunctionSizeCheck>(
         "readability-function-size");
     CheckFactories.registerCheck<IdentifierNamingCheck>(
         "readability-identifier-naming");
     CheckFactories.registerCheck<ImplicitBoolCastCheck>(
         "readability-implicit-bool-cast");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
+    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-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 {
+
+/// Find unnecessary member initializers
+///
+/// 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-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 <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxConstructorDecl(forEachConstructorInitializer(
+          cxxCtorInitializer(
+              isWritten(),
+              withInitializer(cxxConstructExpr().bind("construct")))
+              .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");
+  const auto arguments = Construct->arguments();
+
+  using std::begin;
+  using std::end;
+
+  if (std::find_if(begin(arguments), end(arguments), [](const Expr *expr) {
+        return !expr->isDefaultArgument();
+      }) == end(arguments)) {
+    diag(Init->getSourceLocation(), "initializer for %0 is redundant")
+        << Init->getMember();
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

docs/clang-tidy/checks/list.rst

.. toctree::
    readability-else-after-return
    readability-function-size
    readability-identifier-naming
    readability-implicit-bool-cast
    readability-inconsistent-declaration-parameter-name
    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

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

+.. title:: clang-tidy - readability-redundant-member-init
+
+readability-redundant-member-init
+==================================
+
+Finds unnecessary member initializations.                                                                               
+                                                                                                                        
+Example:                                                                                                               
+                                                                                                                        
+.. code-block:: c++                                                                                                     
+                                                                                                                        
+  // Explicitly initializing the member s is unnecessary.                                                      
+  class Foo {
+    public:
+      Foo() : s() {}
+    private:
+    std::string s;
+  };

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 F1 {
+  F1() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'f' is redundant [readability-redundant-member-init]
+  S f;
+};
+
+struct F2 {
+  F2() : f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'f' is redundant [readability-redundant-member-init]
+  T f;
+};
+
+struct F3 {
+  F3() : f(), g(1), h() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: initializer for 'f' is redundant [readability-redundant-member-init]
+  // CHECK-MESSAGES: :[[@LINE-2]]:21: warning: initializer for 'h' is redundant [readability-redundant-member-init]
+  S f, g, h;
+};
+
+
+struct NF1 {
+  NF1() {}
+  S f;
+};
+
+struct NF2 {
+  NF2() : f(1) {}
+  S f;
+};
+
+struct NF3 {
+  NF3() : f(1) {}
+  T f;
+};
+
+struct NF4 {
+  NF4() : i() {}
+  int i;
+};
+
+struct NF5 {
+  NF5() : i(1) {}
+  int i;
+};