baloghadamsoftware updated this revision to Diff 266117. baloghadamsoftware added a comment.
Updated according to the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71199/new/ https://reviews.llvm.org/D71199 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp @@ -0,0 +1,435 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %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; +}; + +class Simple2 { + int n; + double x; + // CHECK-FIXES: double x{0.0}; + +public: + Simple2() : n (0) { + 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: {{^\ *$}} + } + + 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; + // CHECK-FIXES: int n{0}; + double x; + +public: + Simple3() : x (0.0) { + 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: {{^\ *$}} + } + + 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; + // CHECK-FIXES: int m{1}; + +public: + Complex15() : n(0) { + returning(); + m = 1; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'm' should be initialized in an in-class default member initializer [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 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: {{^\ *$}} + } +}; Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp @@ -0,0 +1,51 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t -- \ +// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-prefer-member-initializer.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() = default; +}; + +class Simple2 { + int n; + double x; + // CHECK-FIXES: double x = 0.0; + +public: + Simple2() : n (0) { + 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: {{^\ *$}} + } + + ~Simple2() = default; +}; + +class Simple3 { + int n; + // CHECK-FIXES: int n = 0; + double x; + +public: + Simple3() : x (0.0) { + 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: {{^\ *$}} + } + + ~Simple3() = default; +}; 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 @@ -138,6 +138,7 @@ `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>`_, `cppcoreguidelines-pro-bounds-array-to-pointer-decay <cppcoreguidelines-pro-bounds-array-to-pointer-decay.html>`_, `cppcoreguidelines-pro-bounds-constant-array-index <cppcoreguidelines-pro-bounds-constant-array-index.html>`_, "Yes" `cppcoreguidelines-pro-bounds-pointer-arithmetic <cppcoreguidelines-pro-bounds-pointer-arithmetic.html>`_, Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-member-initializer.rst @@ -0,0 +1,94 @@ +.. 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. + +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. + +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; + } Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -113,6 +113,12 @@ Flags use of the `C` standard library functions ``memset``, ``memcpy`` and ``memcmp`` and similar derivatives on non-trivial types. +- 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:`llvmlibc-callee-namespace <clang-tidy/checks/llvmlibc-callee-namespace>` check. Index: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h @@ -0,0 +1,37 @@ +//===--- 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); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + + const bool UseAssignment; +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERMEMBERINITIALIZERCHECK_H Index: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp @@ -0,0 +1,233 @@ +//===--- 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" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +static bool isControlStatement(const Stmt *S) { + return isa<IfStmt>(S) || + isa<SwitchStmt>(S) || + isa<ForStmt>(S) || + isa<WhileStmt>(S) || + isa<DoStmt>(S) || + isa<ReturnStmt>(S) || + isa<GotoStmt>(S) || + isa<CXXTryStmt>(S) || + isa<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>(E) || + isa<CharacterLiteral>(E) || + isa<IntegerLiteral>(E) || + isa<FloatingLiteral>(E) || + isa<CXXBoolLiteralExpr>(E) || + isa<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), + UseAssignment(Options.get("UseAssignment", 0) != 0) {} + +void PreferMemberInitializerCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "UseAssignment", UseAssignment); +} + +void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + 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 auto *S: Body->body()) { + if (isControlStatement(S)) + return; + + if (isNoReturnCallStatement(S)) + return; + + const FieldDecl *Field; + const Expr *InitValue; + std::tie(Field, InitValue) = isAssignmentToMemberOf(Class, S); + if (Field) { + if (getLangOpts().CPlusPlus11 && + Ctor->isDefaultConstructor() && + (getLangOpts().CPlusPlus2a || !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 (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 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 @@ -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 @@ "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>( 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 @@ -13,6 +13,7 @@ NarrowingConversionsCheck.cpp NoMallocCheck.cpp OwningMemoryCheck.cpp + PreferMemberInitializerCheck.cpp ProBoundsArrayToPointerDecayCheck.cpp ProBoundsConstantArrayIndexCheck.cpp ProBoundsPointerArithmeticCheck.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits