baloghadamsoftware updated this revision to Diff 233087.
baloghadamsoftware added a comment.
Updated according to some comments.


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

https://reviews.llvm.org/D71199

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/PreferInitializationListCheck.cpp
  clang-tools-extra/clang-tidy/readability/PreferInitializationListCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-prefer-initialization-list.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-prefer-initialization-list.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-prefer-initialization-list.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-prefer-initialization-list.cpp
@@ -0,0 +1,304 @@
+// RUN: %check_clang_tidy %s readability-prefer-initialization-list %t
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+    n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: n should be initialized in the initializer list of the constructor [readability-prefer-initialization-list]
+    x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: x should be initialized in the initializer list of the constructor [readability-prefer-initialization-list]
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n (0) {
+    x = 0.0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: x should be initialized in the initializer list of the constructor [readability-prefer-initialization-list]
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x (0.0) {
+    n = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: n should be initialized in the initializer list of the constructor [readability-prefer-initialization-list]
+  }
+
+  ~Simple3() = default;
+};
+
+static bool dice();
+
+class Complex1 {
+  int n;
+  int m;
+
+public:
+  Complex1() : n(0) {
+    if (dice())
+      m = 1;
+// NO-MESSAGES: initialization of m is nested in a conditional expression
+  }
+
+  ~Complex1() = default;
+};
+
+class Complex2 {
+  int n;
+  int m;
+
+public:
+  Complex2() : n(0) {
+    if (!dice())
+      return;
+    m = 1;
+// NO-MESSAGES: initialization of m follows a conditional expression
+  }
+
+  ~Complex2() = default;
+};
+
+class Complex3 {
+  int n;
+  int m;
+
+public:
+  Complex3() : n(0) {
+    while (dice())
+      m = 1;
+// NO-MESSAGES: initialization of m is nested in a conditional loop
+  }
+
+  ~Complex3() = default;
+};
+
+class Complex4 {
+  int n;
+  int m;
+
+public:
+  Complex4() : n(0) {
+    while (!dice())
+      return;
+    m = 1;
+// NO-MESSAGES: initialization of m follows a conditional loop
+  }
+
+  ~Complex4() = default;
+};
+
+class Complex5 {
+  int n;
+  int m;
+
+public:
+  Complex5() : n(0) {
+    do {
+      m = 1;
+// NO-MESSAGES: initialization of m is nested in a conditional loop
+    } while (dice());
+  }
+
+  ~Complex5() = default;
+};
+
+class Complex6 {
+  int n;
+  int m;
+
+public:
+  Complex6() : n(0) {
+    do {
+      return;
+    } while (!dice());
+    m = 1;
+    // NO-MESSAGES: initialization of m follows a conditional loop
+  }
+
+  ~Complex6() = default;
+};
+
+class Complex7 {
+  int n;
+  int m;
+
+public:
+  Complex7() : n(0) {
+    for (int i = 2; i < 1; ++i) {
+      m = 1;
+    }
+    // NO-MESSAGES: initialization of m is nested into a conditional loop
+  }
+
+  ~Complex7() = default;
+};
+
+class Complex8 {
+  int n;
+  int m;
+
+public:
+  Complex8() : n(0) {
+    for (int i = 0; i < 2; ++i) {
+      return;
+    }
+    m = 1;
+    // NO-MESSAGES: initialization of m follows a conditional loop
+  }
+
+  ~Complex8() = default;
+};
+
+class Complex9 {
+  int n;
+  int m;
+
+public:
+  Complex9() : n(0) {
+    switch (dice()) {
+    case 1:
+      m = 1;
+    // NO-MESSAGES: initialization of m is nested in a conditional expression
+      break;
+    default:
+      break;
+    }
+  }
+
+  ~Complex9() = default;
+};
+
+class Complex10 {
+  int n;
+  int m;
+
+public:
+  Complex10() : n(0) {
+    switch (dice()) {
+    case 1:
+      return;
+      break;
+    default:
+      break;
+    }
+    m = 1;
+    // NO-MESSAGES: initialization of m follows a conditional expression
+  }
+
+  ~Complex10() = default;
+};
+
+class E {};
+void risky(); // may throw
+
+class Complex11 {
+  int n;
+  int m;
+
+public:
+  Complex11() : n(0) {
+    try {
+      risky();
+      m = 1;
+    // NO-MESSAGES: initialization of m follows is nested in a try-block
+    } catch (const E& e) {
+      return;
+    }
+  }
+
+  ~Complex11() = default;
+};
+
+class Complex12 {
+  int n;
+  int m;
+
+public:
+  Complex12() : n(0) {
+    try {
+      risky();
+    } catch (const E& e) {
+      return;
+    }
+    m = 1;
+    // NO-MESSAGES: initialization of m follows a try-block
+  }
+
+  ~Complex12() = default;
+};
+
+class Complex13 {
+  int n;
+  int m;
+
+public:
+  Complex13() : n(0) {
+    return;
+    m = 1;
+    // NO-MESSAGES: initialization of m follows a return statement
+  }
+
+  ~Complex13() = default;
+};
+
+class Complex14 {
+  int n;
+  int m;
+
+public:
+  Complex14() : n(0) {
+    goto X;
+    m = 1;
+    // NO-MESSAGES: initialization of m follows a goto statement
+  X:
+    ;
+  }
+
+  ~Complex14() = default;
+};
+
+void returning();
+
+class Complex15 {
+  int n;
+  int m;
+
+public:
+  Complex15() : n(0) {
+    returning();
+    m = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: m should be initialized in the initializer list of the constructor [readability-prefer-initialization-list]
+  }
+
+  ~Complex15() = default;
+};
+
+[[noreturn]] void not_returning();
+
+class Complex16 {
+  int n;
+  int m;
+
+public:
+  Complex16() : n(0) {
+    not_returning();
+    m = 1;
+    // NO-MESSAGES: initialization of m follows a non-returning function call
+  }
+
+  ~Complex16() = default;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-prefer-initialization-list.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-prefer-initialization-list.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - readability-prefer-initialization-list
+
+readability-prefer-initialization-list
+======================================
+
+Finds member initializations in the constructor body which can be placed into
+the initialization list instead. This does not only improves the readability of
+the code but also positively affects its performance. Class-member assignments
+inside a control statement or following the first control statement are ignored.
+
+Example:
+
+.. code-block:: c++
+
+  class C {
+    int n;
+    int m;
+  public:
+    C() {
+      n = 1;
+      if (dice())
+        return;
+      m = 1;
+    }
+  };
+
+Here ``n`` can be initialized in the constructor initialization list, unlike
+``m``, as ``m``'s initialization follows a control statement (``if``):
+
+.. code-block:: c++
+
+    C() : n(1) {
+      if (dice())
+        return;
+      m = 1;
+    }
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -370,6 +370,7 @@
    readability-misplaced-array-index
    readability-named-parameter
    readability-non-const-parameter
+   readability-prefer-initialization-list
    readability-redundant-access-specifiers
    readability-redundant-control-flow
    readability-redundant-declaration
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -167,6 +167,12 @@
   <clang-tidy/checks/modernize-use-equals-default>` fix no longer adds
   semicolons where they would be redundant.
 
+- New :doc:`readability-prefer-initialization-list
+  <clang-tidy/checks/readability-prefer-initialization-list>` check.
+
+  Finds member initializations in the constructor body which can be placed into
+  the initialization list instead.
+
 - New :doc:`readability-redundant-access-specifiers
   <clang-tidy/checks/readability-redundant-access-specifiers>` check.
 
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -28,6 +28,7 @@
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
+#include "PreferInitializationListCheck.h"
 #include "RedundantAccessSpecifiersCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
@@ -86,6 +87,8 @@
         "readability-misleading-indentation");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
         "readability-misplaced-array-index");
+    CheckFactories.registerCheck<PreferInitializationListCheck>(
+        "readability-prefer-initialization-list");
     CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
         "readability-redundant-access-specifiers");
     CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
Index: clang-tools-extra/clang-tidy/readability/PreferInitializationListCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/PreferInitializationListCheck.h
@@ -0,0 +1,35 @@
+//===--- PreferInitializationListCheck.h - clang-tidy -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_PREFERINITIALIZATIONLISTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_PREFERINITIALIZATIONLISTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds member initializations in the constructor body which can be placed
+/// into the initialization list instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-prefer-initialization-list.html
+class PreferInitializationListCheck : public ClangTidyCheck {
+public:
+  PreferInitializationListCheck(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_PREFERINITIALIZATIONLISTCHECK_H
Index: clang-tools-extra/clang-tidy/readability/PreferInitializationListCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/PreferInitializationListCheck.cpp
@@ -0,0 +1,108 @@
+//===--- PreferInitializationListCheck.cpp - clang-tidy -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "PreferInitializationListCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+static bool isControlStatement(const Stmt *S) {
+  return isa<IfStmt>(S) ||
+         isa<SwitchStmt>(S) ||
+         isa<ForStmt>(S) ||
+         isa<WhileStmt>(S) ||
+         isa<DoStmt>(S) ||
+         isa<ReturnStmt>(S) ||
+         isa<GotoStmt>(S) ||
+         isa<CXXTryStmt>(S);
+}
+
+static bool isNoReturnCallStatement(const Stmt *S) {
+  const auto *Call = dyn_cast<CallExpr>(S);
+  if (!Call)
+    return false;
+
+  const FunctionDecl *Func = Call->getDirectCallee();
+  if (!Func)
+    return false;
+
+  return Func->isNoReturn();
+}
+
+static const FieldDecl *isAssignmentToMemberOf(const RecordDecl *Rec,
+                                               const Stmt *S) {
+  if (const auto *BO = dyn_cast<BinaryOperator>(S)) {
+    if (BO->getOpcode() != BO_Assign)
+      return nullptr;
+
+    const auto *ME = dyn_cast<MemberExpr>(BO->getLHS()->IgnoreParenImpCasts());
+    if (!ME)
+      return nullptr;
+
+    const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl());
+    if (!Field)
+      return nullptr;
+
+    if (isa<CXXThisExpr>(ME->getBase()))
+      return Field;
+  } else if (const auto *COCE = dyn_cast<CXXOperatorCallExpr>(S)) {
+    if (COCE->getOperator() != OO_Equal)
+      return nullptr;
+
+    const auto *ME =
+      dyn_cast<MemberExpr>(COCE->getArg(0)->IgnoreParenImpCasts());
+    if (!ME)
+      return nullptr;
+
+    const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl());
+    if (!Field)
+      return nullptr;
+
+    if (isa<CXXThisExpr>(ME->getBase()))
+      return Field;
+  }
+
+  return nullptr;
+}
+
+void PreferInitializationListCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(cxxConstructorDecl().bind("ctor"), this);
+}
+
+void PreferInitializationListCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+
+  const auto *Body = dyn_cast_or_null<CompoundStmt>(Ctor->getBody());
+  if (!Body)
+    return;
+
+  const CXXRecordDecl *Class = Ctor->getParent();
+
+  for (const auto *S: Body->body()) {
+    if (isControlStatement(S))
+      return;
+
+    if (isNoReturnCallStatement(S))
+      return;
+
+    if (const NamedDecl* Mbr = isAssignmentToMemberOf(Class, S)) {
+      diag(S->getBeginLoc(), "%0 should be initialized in the initializer list"
+           " of the constructor") << Mbr->getName();
+    }
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -21,6 +21,7 @@
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
+  PreferInitializationListCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantAccessSpecifiersCheck.cpp
   RedundantControlFlowCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to