DNS320 created this revision.
DNS320 added reviewers: aaron.ballman, njames93, alexfh.
DNS320 added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai.
DNS320 requested review of this revision.
Herald added a subscriber: cfe-commits.
This check tries to implement C.45
<https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-default>
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?
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104112
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.h
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-init-default-constructors.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-init-default-constructors.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-init-default-constructors.cpp
@@ -0,0 +1,66 @@
+// 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]
+};
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
@@ -145,6 +145,7 @@
`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>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-init-default-constructors.rst
@@ -0,0 +1,26 @@
+.. 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.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,12 @@
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
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -15,6 +15,7 @@
#include "../modernize/UseOverrideCheck.h"
#include "../readability/MagicNumbersCheck.h"
#include "AvoidGotoCheck.h"
+#include "AvoidInitDefaultConstructorsCheck.h"
#include "AvoidNonConstGlobalVariablesCheck.h"
#include "InitVariablesCheck.h"
#include "InterfacesGlobalInitCheck.h"
@@ -48,6 +49,8 @@
"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>(
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -5,6 +5,7 @@
add_clang_library(clangTidyCppCoreGuidelinesModule
AvoidGotoCheck.cpp
+ AvoidInitDefaultConstructorsCheck.cpp
AvoidNonConstGlobalVariablesCheck.cpp
CppCoreGuidelinesTidyModule.cpp
InitVariablesCheck.cpp
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.h
@@ -0,0 +1,38 @@
+//===--- 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
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidInitDefaultConstructorsCheck.cpp
@@ -0,0 +1,45 @@
+//===--- 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
