[clang-tools-extra] 4fc0214 - [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

Adam Balogh via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 21 05:43:53 PDT 2020


Author: Adam Balogh
Date: 2020-09-21T14:42:58+02:00
New Revision: 4fc0214a10140fa77449677e8094ea22d3d17701

URL: https://github.com/llvm/llvm-project/commit/4fc0214a10140fa77449677e8094ea22d3d17701
DIFF: https://github.com/llvm/llvm-project/commit/4fc0214a10140fa77449677e8094ea22d3d17701.diff

LOG: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

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 affects positively its performance.
Class-member assignments inside a control statement or following the
first control statement are ignored.

Differential Revision: https://reviews.llvm.org/D71199

Added: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
    clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
    clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp

Modified: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
    clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
    clang-tools-extra/docs/ReleaseNotes.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index 39c2c552eb73..a9f5b3e0c15b 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -13,6 +13,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
   NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   OwningMemoryCheck.cpp
+  PreferMemberInitializerCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp
   ProBoundsConstantArrayIndexCheck.cpp
   ProBoundsPointerArithmeticCheck.cpp

diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
index 4cb5022888d3..bf613109f0eb 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -22,6 +22,7 @@
 #include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
+#include "PreferMemberInitializerCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "ProBoundsConstantArrayIndexCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
@@ -66,6 +67,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
         "cppcoreguidelines-non-private-member-variables-in-classes");
     CheckFactories.registerCheck<OwningMemoryCheck>(
         "cppcoreguidelines-owning-memory");
+    CheckFactories.registerCheck<PreferMemberInitializerCheck>(
+        "cppcoreguidelines-prefer-member-initializer");
     CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
         "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
     CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(

diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
new file mode 100644
index 000000000000..bc0a3b98ac7a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
@@ -0,0 +1,246 @@
+//===--- PreferMemberInitializerCheck.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 "PreferMemberInitializerCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+static bool isControlStatement(const Stmt *S) {
+  return isa<IfStmt, SwitchStmt, ForStmt, WhileStmt, DoStmt, ReturnStmt,
+             GotoStmt, CXXTryStmt, CXXThrowExpr>(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 bool isLiteral(const Expr *E) {
+  return isa<StringLiteral, CharacterLiteral, IntegerLiteral, FloatingLiteral,
+             CXXBoolLiteralExpr, CXXNullPtrLiteralExpr>(E);
+}
+
+static bool isUnaryExprOfLiteral(const Expr *E) {
+  if (const auto *UnOp = dyn_cast<UnaryOperator>(E))
+    return isLiteral(UnOp->getSubExpr());
+  return false;
+}
+
+static bool shouldBeDefaultMemberInitializer(const Expr *Value) {
+  if (isLiteral(Value) || isUnaryExprOfLiteral(Value))
+    return true;
+
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(Value))
+    return isa<EnumConstantDecl>(DRE->getDecl());
+
+  return false;
+}
+
+static const std::pair<const FieldDecl *, const Expr *>
+isAssignmentToMemberOf(const RecordDecl *Rec, const Stmt *S) {
+  if (const auto *BO = dyn_cast<BinaryOperator>(S)) {
+    if (BO->getOpcode() != BO_Assign)
+      return std::make_pair(nullptr, nullptr);
+
+    const auto *ME = dyn_cast<MemberExpr>(BO->getLHS()->IgnoreParenImpCasts());
+    if (!ME)
+      return std::make_pair(nullptr, nullptr);
+
+    const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl());
+    if (!Field)
+      return std::make_pair(nullptr, nullptr);
+
+    if (isa<CXXThisExpr>(ME->getBase()))
+      return std::make_pair(Field, BO->getRHS()->IgnoreParenImpCasts());
+  } else if (const auto *COCE = dyn_cast<CXXOperatorCallExpr>(S)) {
+    if (COCE->getOperator() != OO_Equal)
+      return std::make_pair(nullptr, nullptr);
+
+    const auto *ME =
+        dyn_cast<MemberExpr>(COCE->getArg(0)->IgnoreParenImpCasts());
+    if (!ME)
+      return std::make_pair(nullptr, nullptr);
+
+    const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl());
+    if (!Field)
+      return std::make_pair(nullptr, nullptr);
+
+    if (isa<CXXThisExpr>(ME->getBase()))
+      return std::make_pair(Field, COCE->getArg(1)->IgnoreParenImpCasts());
+  }
+
+  return std::make_pair(nullptr, nullptr);
+}
+
+PreferMemberInitializerCheck::PreferMemberInitializerCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IsUseDefaultMemberInitEnabled(
+          Context->isCheckEnabled("modernize-use-default-member-init")),
+      UseAssignment(OptionsView("modernize-use-default-member-init",
+                                Context->getOptions().CheckOptions)
+                        .get("UseAssignment", false)) {}
+
+void PreferMemberInitializerCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "UseAssignment", UseAssignment);
+}
+
+void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxConstructorDecl(hasBody(compoundStmt()), unless(isInstantiated()))
+          .bind("ctor"),
+      this);
+}
+
+void PreferMemberInitializerCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+  const auto *Body = cast<CompoundStmt>(Ctor->getBody());
+
+  const CXXRecordDecl *Class = Ctor->getParent();
+  SourceLocation InsertPos;
+  bool FirstToCtorInits = true;
+
+  for (const Stmt *S : Body->body()) {
+    if (S->getBeginLoc().isMacroID()) {
+      StringRef MacroName =
+        Lexer::getImmediateMacroName(S->getBeginLoc(), *Result.SourceManager,
+                                     getLangOpts());
+      if (MacroName.contains_lower("assert"))
+        return;
+    }
+    if (isControlStatement(S))
+      return;
+
+    if (isNoReturnCallStatement(S))
+      return;
+
+    if (const auto *CondOp = dyn_cast<ConditionalOperator>(S)) {
+      if (isNoReturnCallStatement(CondOp->getLHS()) ||
+          isNoReturnCallStatement(CondOp->getRHS()))
+        return;
+    }
+
+    const FieldDecl *Field;
+    const Expr *InitValue;
+    std::tie(Field, InitValue) = isAssignmentToMemberOf(Class, S);
+    if (Field) {
+      if (IsUseDefaultMemberInitEnabled && getLangOpts().CPlusPlus11 &&
+          Ctor->isDefaultConstructor() &&
+          (getLangOpts().CPlusPlus20 || !Field->isBitField()) &&
+          (!isa<RecordDecl>(Class->getDeclContext()) ||
+           !cast<RecordDecl>(Class->getDeclContext())->isUnion()) &&
+          shouldBeDefaultMemberInitializer(InitValue)) {
+        auto Diag =
+            diag(S->getBeginLoc(), "%0 should be initialized in an in-class"
+                                   " default member initializer")
+            << Field;
+
+        SourceLocation FieldEnd =
+            Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
+                                       *Result.SourceManager, getLangOpts());
+        Diag << FixItHint::CreateInsertion(FieldEnd,
+                                           UseAssignment ? " = " : "{")
+             << FixItHint::CreateInsertionFromRange(
+                    FieldEnd,
+                    CharSourceRange(InitValue->getSourceRange(), true))
+             << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? "" : "}");
+
+        SourceLocation SemiColonEnd =
+            Lexer::findNextToken(S->getEndLoc(), *Result.SourceManager,
+                                 getLangOpts())
+                ->getEndLoc();
+        CharSourceRange StmtRange =
+            CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd);
+
+        Diag << FixItHint::CreateRemoval(StmtRange);
+      } else {
+        auto Diag =
+            diag(S->getBeginLoc(), "%0 should be initialized in a member"
+                                   " initializer of the constructor")
+            << Field;
+
+        bool AddComma = false;
+        if (!Ctor->getNumCtorInitializers() && FirstToCtorInits) {
+          SourceLocation BodyPos = Ctor->getBody()->getBeginLoc();
+          SourceLocation NextPos = Ctor->getBeginLoc();
+          do {
+            InsertPos = NextPos;
+            NextPos = Lexer::findNextToken(NextPos, *Result.SourceManager,
+                                           getLangOpts())
+                          ->getLocation();
+          } while (NextPos != BodyPos);
+          InsertPos = Lexer::getLocForEndOfToken(
+              InsertPos, 0, *Result.SourceManager, getLangOpts());
+
+          Diag << FixItHint::CreateInsertion(InsertPos, " : ");
+        } else {
+          bool Found = false;
+          for (const auto *Init : Ctor->inits()) {
+            if (Init->isMemberInitializer()) {
+              if (Result.SourceManager->isBeforeInTranslationUnit(
+                      Field->getLocation(), Init->getMember()->getLocation())) {
+                InsertPos = Init->getSourceLocation();
+                Found = true;
+                break;
+              }
+            }
+          }
+
+          if (!Found) {
+            if (Ctor->getNumCtorInitializers()) {
+              InsertPos = Lexer::getLocForEndOfToken(
+                  (*Ctor->init_rbegin())->getSourceRange().getEnd(), 0,
+                  *Result.SourceManager, getLangOpts());
+            }
+            Diag << FixItHint::CreateInsertion(InsertPos, ", ");
+          } else {
+            AddComma = true;
+          }
+        }
+        Diag << FixItHint::CreateInsertion(InsertPos, Field->getName())
+             << FixItHint::CreateInsertion(InsertPos, "(")
+             << FixItHint::CreateInsertionFromRange(
+                    InsertPos,
+                    CharSourceRange(InitValue->getSourceRange(), true))
+             << FixItHint::CreateInsertion(InsertPos, ")");
+        if (AddComma)
+          Diag << FixItHint::CreateInsertion(InsertPos, ", ");
+
+        SourceLocation SemiColonEnd =
+            Lexer::findNextToken(S->getEndLoc(), *Result.SourceManager,
+                                 getLangOpts())
+                ->getEndLoc();
+        CharSourceRange StmtRange =
+            CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd);
+
+        Diag << FixItHint::CreateRemoval(StmtRange);
+        FirstToCtorInits = false;
+      }
+    }
+  }
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
new file mode 100644
index 000000000000..dbef7c98d8e3
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h
@@ -0,0 +1,41 @@
+//===--- PreferMemberInitializerCheck.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_CPPCOREGUIDELINES_PREFERMEMBERINITIALIZERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERMEMBERINITIALIZERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// 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/cppcoreguidelines-prefer-member-initializer.html
+class PreferMemberInitializerCheck : public ClangTidyCheck {
+public:
+  PreferMemberInitializerCheck(StringRef Name, ClangTidyContext *Context);
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+  const bool IsUseDefaultMemberInitEnabled;
+  const bool UseAssignment;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERMEMBERINITIALIZERCHECK_H

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 563c0eced92e..befe338b91c6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -85,6 +85,12 @@ New checks
   Finds structs that are inefficiently packed or aligned, and recommends
   packing and/or aligning of said structs as needed.
 
+- New :doc:`cppcoreguidelines-prefer-member-initializer
+  <clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
+
+  Finds member initializations in the constructor body which can be placed into
+  the initialization list instead.
+
 - New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
   <clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc>` check.
 

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
new file mode 100644
index 000000000000..c8a4c741fd1e
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst
@@ -0,0 +1,103 @@
+.. title:: clang-tidy - cppcoreguidelines-prefer-member-initializer
+
+cppcoreguidelines-prefer-member-initializer
+===========================================
+
+Finds member initializations in the constructor body which can be  converted
+into member initializers of the constructor instead. This 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.
+
+This check implements `C.49 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c49-prefer-initialization-to-assignment-in-constructors>`_ from the CppCoreGuidelines.
+
+If the language version is `C++ 11` or above, the constructor is the default
+constructor of the class, the field is not a bitfield (only in case of earlier
+language version than `C++ 20`), furthermore the assigned value is a literal,
+negated literal or ``enum`` constant then the preferred place of the
+initialization is at the class member declaration.
+
+This latter rule is `C.48 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_ from CppCoreGuidelines.
+
+Please note, that this check does not enforce this latter rule for
+initializations already implemented as member initializers. For that purpose
+see check `modernize-use-default-member-init <modernize-use-default-member-init.html>`_.
+
+Example 1
+---------
+
+.. code-block:: c++
+
+  class C {
+    int n;
+    int m;
+  public:
+    C() {
+      n = 1; // Literal in default constructor
+      if (dice())
+        return;
+      m = 1;
+    }
+  };
+
+Here ``n`` can be initialized using a default member initializer, unlike
+``m``, as ``m``'s initialization follows a control statement (``if``):
+
+.. code-block:: c++
+
+  class C {
+    int n{1};
+    int m;
+  public:
+    C() {
+      if (dice())
+        return;
+      m = 1;
+    }
+
+Example 2
+---------
+
+.. code-block:: c++
+
+  class C {
+    int n;
+    int m;
+  public:
+    C(int nn, int mm) {
+      n = nn; // Neither default constructor nor literal
+      if (dice())
+        return;
+      m = mm;
+    }
+  };
+
+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(int nn, int mm) : n(nn) {
+    if (dice())
+      return;
+    m = mm;
+  }
+
+.. option:: UseAssignment
+
+   If this option is set to non-zero (default is `0`), the check will initialize
+   members with an assignment. In this case the fix of the first example looks
+   like this:
+
+.. code-block:: c++
+
+  class C {
+    int n = 1;
+    int m;
+  public:
+    C() {
+      if (dice())
+        return;
+      m = 1;
+    }
+  };

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
new file mode 100644
index 000000000000..dc6cb7606a0d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \
+// RUN: -config="{CheckOptions: [{key: modernize-use-default-member-init.UseAssignment, value: 1}]}"
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n = 0;
+  double x;
+  // CHECK-FIXES: double x = 0.0;
+
+public:
+  Simple1() {
+    n = 0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x = 0.0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+    // CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+    n = nn;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x = xx;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
new file mode 100644
index 000000000000..fe5bb7c3bb98
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init.cpp
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t
+
+class Simple1 {
+  int n;
+  // CHECK-FIXES: int n{0};
+  double x;
+  // CHECK-FIXES: double x{0.0};
+
+public:
+  Simple1() {
+    n = 0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x = 0.0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+    // CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+    n = nn;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x = xx;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
new file mode 100644
index 000000000000..b5c04c32c9fa
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -0,0 +1,490 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t -- -- -fcxx-exceptions
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+    unsigned int __line, __const char *__function)
+     __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+class Simple1 {
+  int n;
+  double x;
+
+public:
+  Simple1() {
+    // CHECK-FIXES: Simple1() : n(0), x(0.0) {
+    n = 0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x = 0.0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple1(int nn, double xx) {
+    // CHECK-FIXES: Simple1(int nn, double xx) : n(nn), x(xx) {
+    n = nn;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x = xx;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple1() = default;
+};
+
+class Simple2 {
+  int n;
+  double x;
+
+public:
+  Simple2() : n(0) {
+    // CHECK-FIXES: Simple2() : n(0), x(0.0) {
+    x = 0.0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple2(int nn, double xx) : n(nn) {
+    // CHECK-FIXES: Simple2(int nn, double xx) : n(nn), x(xx) {
+    x = xx;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple2() = default;
+};
+
+class Simple3 {
+  int n;
+  double x;
+
+public:
+  Simple3() : x(0.0) {
+    // CHECK-FIXES: Simple3() : n(0), x(0.0) {
+    n = 0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  Simple3(int nn, double xx) : x(xx) {
+    // CHECK-FIXES: Simple3(int nn, double xx) : n(nn), x(xx) {
+    n = nn;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple3() = default;
+};
+
+int something_int();
+double something_double();
+
+class Simple4 {
+  int n;
+
+public:
+  Simple4() {
+    // CHECK-FIXES: Simple4() : n(something_int()) {
+    n = something_int();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Simple4() = 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 {};
+int 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) {
+    // CHECK-FIXES: Complex15() : n(0), m(1) {
+    returning();
+    m = 1;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'm' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~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;
+};
+
+class Complex17 {
+  int n;
+  int m;
+
+public:
+  Complex17() : n(0) {
+    throw 1;
+    m = 1;
+    // NO-MESSAGES: initialization of 'm' follows a 'throw' statement;
+  }
+
+  ~Complex17() = default;
+};
+
+class Complex18 {
+  int n;
+
+public:
+  Complex18() try {
+    n = risky();
+    // NO-MESSAGES: initialization of 'n' in a 'try' body;
+  } catch (const E& e) {
+    n = 0;
+  }
+
+  ~Complex18() = default;
+};
+
+class Complex19 {
+  int n;
+public:
+  Complex19() {
+    // CHECK-FIXES: Complex19() : n(0) {
+    n = 0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  explicit Complex19(int) {
+    // CHECK-FIXES: Complex19(int) : n(12) {
+    n = 12;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+
+  ~Complex19() = default;
+};
+
+class Complex20 {
+  int n;
+  int m;
+
+public:
+  Complex20(int k) : n(0) {
+    assert(k > 0);
+    m = 1;
+    // NO-MESSAGES: initialization of 'm' follows an assertion
+  }
+
+  ~Complex20() = default;
+};
+
+class VeryComplex1 {
+  int n1, n2, n3;
+  double x1, x2, x3;
+  int n4, n5, n6;
+  double x4, x5, x6;
+
+  VeryComplex1() : n3(something_int()), x3(something_double()),
+                   n5(something_int()), x4(something_double()),
+                   x5(something_double()) {
+    // CHECK-FIXES: VeryComplex1() : n2(something_int()), n1(something_int()), n3(something_int()), x2(something_double()), x1(something_double()), x3(something_double()),
+    // CHECK-FIXES:                  n4(something_int()), n5(something_int()), n6(something_int()), x4(something_double()),
+    // CHECK-FIXES:                  x5(something_double()), x6(something_double()) {
+
+// FIXME: Order of elements on the constructor initializer list should match
+//        the order of the declaration of the fields. Thus the correct fixes
+//        should look like these:
+//
+    // C ECK-FIXES: VeryComplex1() : n2(something_int()), n1(something_int()), n3(something_int()), x2(something_double()), x1(something_double()), x3(something_double()),
+    // C ECK-FIXES:                  n4(something_int()), n5(something_int()), n6(something_int()), x4(something_double()),
+    // C ECK-FIXES:                  x5(something_double()), x6(something_double()) {
+//
+//        However, the Diagnostics Engine processes fixes in the order of the
+//        diagnostics and insertions to the same position are handled in left to
+//        right order thus in the case two adjacent fields are initialized
+//        inside the constructor in reverse order the provided fix is a
+//        constructor initializer list that does not match the order of the
+//        declaration of the fields.
+
+    x2 = something_double();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x2' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    n2 = something_int();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n2' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x6 = something_double();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x6' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    x1 = something_double();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'x1' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    n6 = something_int();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n6' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    n1 = something_int();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n1' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+    n4 = something_int();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'n4' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  }
+};
+
+struct Outside {
+  int n;
+  double x;
+  Outside();
+};
+
+Outside::Outside() {
+    // CHECK-FIXES: Outside::Outside() : n(1), x(1.0) {
+  n = 1;
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'n' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+  x = 1.0;
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
+    // CHECK-FIXES: {{^\ *$}}
+}


        


More information about the cfe-commits mailing list