[clang-tidy] cppcoreguidelines-avoid-init-default-constructors: a new check

[DNS320]

Summary

This check tries to implement C.45 of the C++ Core Guidelines.
During the implementation of the current state of this check, it was found that multiple functionalities, which the check also could use, already exists in other clang-tidy checks. These are:

• Lookup if the body of a constructor (only) initialize class members (cppcoreguidelines-prefer-member-initializer)
• Set a constructor =default if possible (hicpp-use-equals-default)
• If possible, move/change constructor list initializers to class member initializers (modernize-use-default-member-init)

I would like to ask how I could proceed, as reimplementing already existing functionality of other checks might not be preferred, because of reasons like duplication and maintainability. But I think it would be beneficial if a check exists that combines the mentioned functionalities under the cppcoreguidelines module.

Generally, do you see a need for this check at all or is the activation of the three checks mentioned above the way to go to implicitly enforce C.45?

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.h

28

Shouldn't this check be enabled in C++11 or newer?

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst

6

Please synchronize this statement with Release Notes.

[Eugene.Zelenko]

What about bit filed members - their initialization is supported only in C++20.

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.h

+//===--- AvoidInitDefaultConstructorsCheck.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_AVOIDINITDEFAULTCONSTRUCTORSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDINITDEFAULTCONSTRUCTORSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Finds constructors which just initialize class members, so they can be
+/// replaced with in-class member initializers.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.html
+class AvoidInitDefaultConstructorsCheck : public ClangTidyCheck {
+public:
+  AvoidInitDefaultConstructorsCheck(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;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDINITDEFAULTCONSTRUCTORSCHECK_H

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.cpp

+//===--- AvoidInitDefaultConstructorsCheck.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 "AvoidInitDefaultConstructorsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void AvoidInitDefaultConstructorsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxConstructorDecl(
+          hasBody(compoundStmt(unless(hasAnySubstatement(anything())))),
+          isDefaultConstructor(),
+          unless(hasAnyConstructorInitializer(cxxCtorInitializer(
+              withInitializer(hasDescendant(declRefExpr(to(parmVarDecl()))))))),
+          hasAnyConstructorInitializer(
+              forField(unless(hasInClassInitializer(anything())))))
+          .bind("Constructor"),
+      this);
+}
+
+void AvoidInitDefaultConstructorsCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedConstructor =
+      Result.Nodes.getNodeAs<CXXConstructorDecl>("Constructor");
+
+  diag(MatchedConstructor->getLocation(),
+       "Default constructors like %0 can be omitted. Use in-class member "
+       "initializers instead")
+      << MatchedConstructor;
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

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

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   AvoidGotoCheck.cpp
+  AvoidInitDefaultConstructorsCheck.cpp
   AvoidNonConstGlobalVariablesCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InitVariablesCheck.cpp
   InterfacesGlobalInitCheck.cpp
   MacroUsageCheck.cpp
   NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   OwningMemoryCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
+#include "AvoidInitDefaultConstructorsCheck.h"
 #include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "MacroUsageCheck.h"
 #include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
 #include "PreferMemberInitializerCheck.h"

 /// A module containing checks of the C++ Core Guidelines
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
         "cppcoreguidelines-avoid-c-arrays");
     CheckFactories.registerCheck<AvoidGotoCheck>(
         "cppcoreguidelines-avoid-goto");
+    CheckFactories.registerCheck<AvoidInitDefaultConstructorsCheck>(
+        "cppcoreguidelines-avoid-init-default-constructors");
     CheckFactories.registerCheck<readability::MagicNumbersCheck>(
         "cppcoreguidelines-avoid-magic-numbers");
     CheckFactories.registerCheck<AvoidNonConstGlobalVariablesCheck>(
         "cppcoreguidelines-avoid-non-const-global-variables");
     CheckFactories.registerCheck<modernize::UseOverrideCheck>(
         "cppcoreguidelines-explicit-virtual-functions");
     CheckFactories.registerCheck<InitVariablesCheck>(
         "cppcoreguidelines-init-variables");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`cppcoreguidelines-prefer-member-initializer
   Finds member initializations in the constructor body which can be placed into
   the initialization list instead.
 
 - New :doc:`bugprone-unhandled-exception-at-new
   <clang-tidy/checks/bugprone-unhandled-exception-at-new>` check.
 
   Finds calls to ``new`` with missing exception handler for ``std::bad_alloc``.
 
+- New :doc:`cppcoreguidelines-avoid-init-default-constructors
+  <clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors>` check.
+
+  Finds constructors which just initialize class members, so they can be
+  replaced with in-class member initializers.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-pos47-c
   <clang-tidy/checks/cert-pos47-c>` to
   :doc:`concurrency-thread-canceltype-asynchronous
   <clang-tidy/checks/concurrency-thread-canceltype-asynchronous>` was added.
 

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst

+.. title:: clang-tidy - cppcoreguidelines-avoid-init-default-constructors
+
+cppcoreguidelines-avoid-init-default-constructors
+==================================================================
+
+This check flags constructors which are only initializing class members.
+Such constructors can be omitted, because the initialization could be
+done with in-class member initializers.
+Constructors implicitly generated by the compiler could be more efficient.
+Furthermore, removing unnecessary code improves readability.
+
+.. code-block:: c++
+
+  class C {
+    int z;
+
+    C() : z{1} {} // Can be omitted
+  };
+
+  // Better
+  class C {
+    int z{1};
+  };
+
+This check implements rule `C.45 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-default>`_  
+of the C++ Core Guidelines.

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

.. csv-table::
    `clang-analyzer-osx.cocoa.NonNilReturnValue <clang-analyzer-osx.cocoa.NonNilReturnValue.html>`_,
    `clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak <clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak.html>`_,
    `clang-analyzer-valist.CopyToSelf <clang-analyzer-valist.CopyToSelf.html>`_,
    `clang-analyzer-valist.Uninitialized <clang-analyzer-valist.Uninitialized.html>`_,
    `clang-analyzer-valist.Unterminated <clang-analyzer-valist.Unterminated.html>`_,
    `concurrency-mt-unsafe <concurrency-mt-unsafe.html>`_,
    `concurrency-thread-canceltype-asynchronous <concurrency-thread-canceltype-asynchronous.html>`_,
    `cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
+   `cppcoreguidelines-avoid-init-default-constructors <cppcoreguidelines-avoid-init-default-constructors.html>`_,
    `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_,
    `cppcoreguidelines-init-variables <cppcoreguidelines-init-variables.html>`_, "Yes"
    `cppcoreguidelines-interfaces-global-init <cppcoreguidelines-interfaces-global-init.html>`_,
    `cppcoreguidelines-macro-usage <cppcoreguidelines-macro-usage.html>`_,
    `cppcoreguidelines-narrowing-conversions <cppcoreguidelines-narrowing-conversions.html>`_,
    `cppcoreguidelines-no-malloc <cppcoreguidelines-no-malloc.html>`_,
    `cppcoreguidelines-owning-memory <cppcoreguidelines-owning-memory.html>`_,
    `cppcoreguidelines-prefer-member-initializer <cppcoreguidelines-prefer-member-initializer.html>`_, "Yes"

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-init-default-constructors.cpp

+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-init-default-constructors %t
+
+class NoDefaultCtor {
+  int z;
+
+  NoDefaultCtor(int a) : z{1} {}
+};
+
+class HasBody {
+  int z;
+
+  HasBody() : z{1} { ; }
+};
+
+class OneParameterOneInitList {
+  int z;
+  int y;
+
+  OneParameterOneInitList(int a = 1) : z{a}, y{1} {}
+};
+
+// Test needed because class member initializations are listed as ``cxxCtorInitializer`` in clang's AST
+class OnlyInClassMemberInit {
+  int z{1};
+  int y = 1;
+
+  OnlyInClassMemberInit() {}
+};
+
+// User may wants explicitly overwrite the class member value in the constructor
+class ExplicitOverwrite {
+  int z{1};
+
+  ExplicitOverwrite() : z{2} {}
+};
+
+class HasBodyInit {
+  int z;
+
+  HasBodyInit() {
+    z = 1;
+  }
+};
+
+class OneInitList {
+  int z;
+
+  OneInitList() : z{1} {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Default constructors like 'OneInitList' can be omitted. Use in-class member initializers instead [cppcoreguidelines-avoid-init-default-constructors]
+};
+
+class OneDirectInit {
+  int z;
+
+  OneDirectInit() : z(1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Default constructors like 'OneDirectInit' can be omitted. Use in-class member initializers instead [cppcoreguidelines-avoid-init-default-constructors]
+};
+
+class OneInitListOneDirectInit {
+  int z;
+  int y;
+  int w;
+
+  OneInitListOneDirectInit() : z{1}, y(2) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Default constructors like 'OneInitListOneDirectInit' can be omitted. Use in-class member initializers instead [cppcoreguidelines-avoid-init-default-constructors]
+};