[clang-tidy] The first PR our of many PRs for the "Initialized Class Members" check.

clang-tools-extra/clang-tidy/google/CMakeLists.txt

@@ -8,6 +8,7 @@ add_clang_library(clangTidyGoogleModule
   AvoidNSObjectNewCheck.cpp
   AvoidThrowingObjCExceptionCheck.cpp
   AvoidUnderscoreInGoogletestNameCheck.cpp
+  CppInitClassMembersCheck.cpp
   DefaultArgumentsCheck.cpp
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp

clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.cpp

@@ -0,0 +1,107 @@
+//===--- CppInitClassMembersCheck.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 <string>
+
+#include "CppInitClassMembersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::google {
+
+namespace {
+
+// Matches records that have a default constructor.
+AST_MATCHER(CXXRecordDecl, hasDefaultConstructor) {
+  return Node.hasDefaultConstructor();
+}
+
+// Returns the names of `Fields` in a comma separated string.
+std::string
+toCommaSeparatedString(const ArrayRef<const FieldDecl *> &Fields) {
+  std::string Buffer;
+  llvm::raw_string_ostream OS(Buffer);
+  llvm::interleave(
+      Fields, OS, [&OS](const FieldDecl *Decl) { OS << Decl->getName(); },
+      ", ");
+  return Buffer;
+}
+
+// Returns `true` for types that have uninitialized values by default. For
+// example, returns `true` for `int` because an uninitialized `int` field or
+// local variable can contain uninitialized values.
+bool isDefaultValueUninitialized(QualType Ty) {
+  if (Ty.isNull())
+    return false;
+
+  // FIXME: For now, this check focuses on several allowlisted types. We will
+  // expand coverage in future.
+  return Ty->isIntegerType() || Ty->isBooleanType();
+}
+
+} // anonymous namespace
+
+void CppInitClassMembersCheck::checkMissingMemberInitializer(
+    ASTContext &Context, const CXXRecordDecl &ClassDecl,
+    const CXXConstructorDecl *Ctor) {
+  SmallVector<const FieldDecl *, 16> FieldsToReport;
+
+  for (const FieldDecl *F : ClassDecl.fields()) {
+    if (isDefaultValueUninitialized(F->getType()) &&
+        !F->hasInClassInitializer())
+      FieldsToReport.push_back(F);
+  }
+
+  if (FieldsToReport.empty())
+    return;
+
+  DiagnosticBuilder Diag =
+      diag(Ctor ? Ctor->getBeginLoc() : ClassDecl.getLocation(),
+           "%select{these fields should be initialized|constructor should "
+           "initialize these fields}0: %1")
+      << (Ctor != nullptr) << toCommaSeparatedString(FieldsToReport);
+
+  // FIXME: generate fixes.
+}
+
+void CppInitClassMembersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(cxxConstructorDecl(isDefinition(), isDefaultConstructor(),
+                                        unless(isUserProvided()))
+                         .bind("ctor"),
+                     this);
+
+  Finder->addMatcher(cxxRecordDecl(isDefinition(), hasDefaultConstructor(),
+                                   unless(isInstantiated()),
+                                   unless(has(cxxConstructorDecl())))
+                         .bind("record"),
+                     this);
+}
+
+void CppInitClassMembersCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor")) {
+    checkMissingMemberInitializer(*Result.Context, *Ctor->getParent(), Ctor);
+  } else if (const auto *Record =
+                 Result.Nodes.getNodeAs<CXXRecordDecl>("record")) {
+    // For a record, perform the same action as for a constructor. However,
+    // emit the diagnostic for the record, not for the constructor.
+    checkMissingMemberInitializer(*Result.Context, *Record, nullptr);
+  }
+}
+
+} // namespace clang::tidy::google
+

clang-tools-extra/clang-tidy/google/CppInitClassMembersCheck.h

@@ -0,0 +1,52 @@
+//===--- CppInitClassMembersCheck.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_GOOGLE_CPPINITCLASSMEMBERSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_CPPINITCLASSMEMBERSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::google {
+
+/// Checks that class members are initialized in constructors (implicitly or
+/// explicitly). Reports constructors or classes where class members are not
+/// initialized. The goal of this check is to eliminate UUM (Use of
+/// Uninitialized Memory) bugs caused by uninitialized class members.
+///
+/// This check is under active development: the check authors made a few
+/// commits and are actively working on more commits. Users who want a mature
+/// and stable check should not use this check yet.
+///
+/// This check is different from ProTypeMemberInitCheck in that this check
+/// attempts to eliminate UUMs as a bug class, at the expense of false
+/// positives.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/google/cpp-init-class-members.html
+class CppInitClassMembersCheck : public ClangTidyCheck {
+public:
+  CppInitClassMembersCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  // Issue a diagnostic for any constructor that does not initialize all member
+  // variables. If the record does not have a constructor (`Ctor` is `nullptr`),
+  // the diagnostic is for the record.
+  void checkMissingMemberInitializer(ASTContext &Context,
+                                     const CXXRecordDecl &ClassDecl,
+                                     const CXXConstructorDecl *Ctor);
+};
+
+} // namespace clang::tidy::google
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_CPPINITCLASSMEMBERSCHECK_H

clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp

@@ -16,6 +16,7 @@
 #include "AvoidNSObjectNewCheck.h"
 #include "AvoidThrowingObjCExceptionCheck.h"
 #include "AvoidUnderscoreInGoogletestNameCheck.h"
+#include "CppInitClassMembersCheck.h"
 #include "DefaultArgumentsCheck.h"
 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
@@ -43,6 +44,8 @@ class GoogleModule : public ClangTidyModule {
         "google-build-namespaces");
     CheckFactories.registerCheck<build::UsingNamespaceDirectiveCheck>(
         "google-build-using-namespace");
+    CheckFactories.registerCheck<CppInitClassMembersCheck>(
+        "google-cpp-init-class-members");
     CheckFactories.registerCheck<DefaultArgumentsCheck>(
         "google-default-arguments");
     CheckFactories.registerCheck<ExplicitConstructorCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -148,6 +148,12 @@ New checks
   Flags coroutines that suspend while a lock guard is in scope at the
   suspension point.
 
+- New :doc:`google-cpp-init-class-members
+  <clang-tidy/checks/google/cpp-init-class-members>` check.
+
+  Checks that class members are initialized in constructors (implicitly or
+  explicitly).
+
 - New :doc:`modernize-use-constraints
   <clang-tidy/checks/modernize/use-constraints>` check.
 

clang-tools-extra/docs/clang-tidy/checks/google/cpp-init-class-members.rst

@@ -0,0 +1,34 @@
+.. title:: clang-tidy - google-cpp-init-class-members
+
+google-cpp-init-class-members
+=============================
+
+Checks that class members are initialized in constructors (implicitly or
+explicitly). Reports constructors or classes where class members are not
+initialized. The goal of this check is to eliminate UUM (Use of
+Uninitialized Memory) bugs caused by uninitialized class members.
+
+This check is under active development: the check authors made a few commits
+and are actively working on more commits. Users who want a mature and stable
+check should not use this check yet.
+
+This check is different from ProTypeMemberInitCheck in that this check
+attempts to eliminate UUMs as a bug class, at the expense of false
+positives. The authors of this check will add more documentation about the
+differences with ProTypeMemberInitCheck as the check evolves.
+
+For now, this check reports `X` in the following two patterns:
+
+.. code-block:: c++
+  class SomeClass {
+  public:
+    SomeClass() = default;
+
+  private:
+    int X;
+  };
+
+.. code-block:: c++
+  struct SomeStruct {
+    int X;
+  };

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

@@ -210,6 +210,7 @@ Clang-Tidy Checks
    :doc:`google-build-explicit-make-pair <google/build-explicit-make-pair>`,
    :doc:`google-build-namespaces <google/build-namespaces>`,
    :doc:`google-build-using-namespace <google/build-using-namespace>`,
+   :doc:`google-cpp-init-class-members <google/cpp-init-class-members>`, "Yes"
    :doc:`google-default-arguments <google/default-arguments>`,
    :doc:`google-explicit-constructor <google/explicit-constructor>`, "Yes"
    :doc:`google-global-names-in-headers <google/global-names-in-headers>`,

clang-tools-extra/test/clang-tidy/checkers/google/cpp-init-class-members.cpp

@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s google-cpp-init-class-members %t
+
+// TODO: The following issues are not yet handled by the check and are not
+// covered by the tests. We may decide that the check needs to report some of
+// these cases and should not report some the remaining cases. This exact
+// decision is also part of the TODO:
+//  - Deleted constructor
+//  - Inheritance
+//  - Struct with anonymous members
+//  - Struct with anonymous union members
+//  - Unnamed structs
+//  - Nested structs
+//  - etc
+
+class PositiveDefaultedDefaultConstructor {
+public:
+  PositiveDefaultedDefaultConstructor() = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor should initialize these fields: X
+
+private:
+  int X;
+};
+
+class PositiveDefaultedDefaultConstructorWithInitializedField {
+public:
+  PositiveDefaultedDefaultConstructorWithInitializedField() = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor should initialize these fields: X
+
+private:
+  int X;
+  int Y = 4; // no-warning
+};
+
+class Helper {
+ public:
+  Helper(int x) : X(x) {}
+
+ private:
+  int X;
+};
+
+class PositiveDefaultedConstructorObjectAndPrimitive {
+ public:
+  PositiveDefaultedConstructorObjectAndPrimitive() = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor should initialize these fields: Y
+
+  Helper* GetHelper() { return &X; }
+
+  void SetY(bool enabled) { Y = enabled; }
+
+  bool IsY() { return Y; }
+
+ private:
+  Helper X;
+  bool Y;
+};
+
+struct PositiveStruct {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: these fields should be initialized: X, Y
+  int X;
+  int Y;
+};
+
+struct PositiveStructWithInitializedField {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: these fields should be initialized: Y
+  int X = 3; // no-warning
+  int Y;
+};