[clang-tools-extra] [clang-tidy] The first PR our of many PRs for the "Initialized Class Members" check. (PR #65189)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 8 12:17:15 PDT 2023


================
@@ -0,0 +1,105 @@
+//===--- 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 SmallVector<const FieldDecl *, 16> &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 field types that should be reported (if additional
+// conditions are also met). For example, returns `true` for `int` because an
+// uninitialized `int` field can contain uninitialized values.
+bool shouldReportThisFieldType(QualType Ty) {
+  if (Ty.isNull())
+    return false;
+
+  // FIXME: For now, this checker 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 (shouldReportThisFieldType(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(),
----------------
PiotrZSL wrote:

The thing is that sometimes constructor declarations wont be visible in AST until they actually used first time, this may happen for implicit constructors.

In such case, `hasDefaultConstructor` may return true, but `unless(has(cxxConstructorDecl()))` may return false or true, depend if constructor is used or not. I assume that what you wanted to check is to check if default constructor is implicit.  Other thing that instead of using "has" it should be "hasMethod".
And here you could use things like `unless(has(cxxConstructorDecl(unless(isImplicit()))))`.


https://github.com/llvm/llvm-project/pull/65189


More information about the cfe-commits mailing list