diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -12,6 +12,7 @@ #include "../misc/NonPrivateMemberVariablesInClassesCheck.h" #include "../misc/UnconventionalAssignOperatorCheck.h" #include "../modernize/AvoidCArraysCheck.h" +#include "../modernize/UseDefaultMemberInitCheck.h" #include "../modernize/UseOverrideCheck.h" #include "../readability/MagicNumbersCheck.h" #include "AvoidCapturingLambdaCoroutinesCheck.h" @@ -110,6 +111,8 @@ CheckFactories.registerCheck( "cppcoreguidelines-special-member-functions"); CheckFactories.registerCheck("cppcoreguidelines-slicing"); + CheckFactories.registerCheck( + "cppcoreguidelines-use-default-member-init"); CheckFactories.registerCheck( "cppcoreguidelines-c-copy-assignment-signature"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp @@ -176,139 +176,98 @@ const Expr *InitValue; std::tie(Field, InitValue) = isAssignmentToMemberOf(Class, S, Ctor); if (Field) { - if (IsUseDefaultMemberInitEnabled && getLangOpts().CPlusPlus11 && - Ctor->isDefaultConstructor() && - (getLangOpts().CPlusPlus20 || !Field->isBitField()) && - !Field->hasInClassInitializer() && - (!isa(Class->getDeclContext()) || - !cast(Class->getDeclContext())->isUnion()) && - shouldBeDefaultMemberInitializer(InitValue)) { - - bool InvalidFix = false; - SourceLocation FieldEnd = - Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0, - *Result.SourceManager, getLangOpts()); - InvalidFix |= FieldEnd.isInvalid() || FieldEnd.isMacroID(); - SourceLocation SemiColonEnd; - if (auto NextToken = Lexer::findNextToken( - S->getEndLoc(), *Result.SourceManager, getLangOpts())) - SemiColonEnd = NextToken->getEndLoc(); - else - InvalidFix = true; - auto Diag = - diag(S->getBeginLoc(), "%0 should be initialized in an in-class" - " default member initializer") - << Field; - if (InvalidFix) + StringRef InsertPrefix = ""; + bool HasInitAlready = false; + SourceLocation InsertPos; + SourceRange ReplaceRange; + bool AddComma = false; + bool InvalidFix = false; + unsigned Index = Field->getFieldIndex(); + const CXXCtorInitializer *LastInListInit = nullptr; + for (const CXXCtorInitializer *Init : Ctor->inits()) { + if (!Init->isWritten() || Init->isInClassMemberInitializer()) continue; - CharSourceRange StmtRange = - CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd); - - SmallString<128> Insertion( - {UseAssignment ? " = " : "{", - Lexer::getSourceText( - CharSourceRange(InitValue->getSourceRange(), true), - *Result.SourceManager, getLangOpts()), - UseAssignment ? "" : "}"}); - - Diag << FixItHint::CreateInsertion(FieldEnd, Insertion) - << FixItHint::CreateRemoval(StmtRange); - - } else { - StringRef InsertPrefix = ""; - bool HasInitAlready = false; - SourceLocation InsertPos; - SourceRange ReplaceRange; - bool AddComma = false; - bool InvalidFix = false; - unsigned Index = Field->getFieldIndex(); - const CXXCtorInitializer *LastInListInit = nullptr; - for (const CXXCtorInitializer *Init : Ctor->inits()) { - if (!Init->isWritten() || Init->isInClassMemberInitializer()) - continue; - if (Init->getMember() == Field) { - HasInitAlready = true; - if (isa(Init->getInit())) - InsertPos = Init->getRParenLoc(); - else { - ReplaceRange = Init->getInit()->getSourceRange(); - } - break; + if (Init->getMember() == Field) { + HasInitAlready = true; + if (isa(Init->getInit())) + InsertPos = Init->getRParenLoc(); + else { + ReplaceRange = Init->getInit()->getSourceRange(); } - if (Init->isMemberInitializer() && - Index < Init->getMember()->getFieldIndex()) { - InsertPos = Init->getSourceLocation(); - // There are initializers after the one we are inserting, so add a - // comma after this insertion in order to not break anything. - AddComma = true; - break; - } - LastInListInit = Init; + break; } - if (HasInitAlready) { - if (InsertPos.isValid()) - InvalidFix |= InsertPos.isMacroID(); - else - InvalidFix |= ReplaceRange.getBegin().isMacroID() || - ReplaceRange.getEnd().isMacroID(); - } else { - if (InsertPos.isInvalid()) { - if (LastInListInit) { - InsertPos = Lexer::getLocForEndOfToken( - LastInListInit->getRParenLoc(), 0, *Result.SourceManager, - getLangOpts()); - // Inserting after the last constructor initializer, so we need a - // comma. - InsertPrefix = ", "; - } else { - InsertPos = Lexer::getLocForEndOfToken( - Ctor->getTypeSourceInfo() - ->getTypeLoc() - .getAs() - .getLocalRangeEnd(), - 0, *Result.SourceManager, getLangOpts()); - - // If this is first time in the loop, there are no initializers so - // `:` declares member initialization list. If this is a - // subsequent pass then we have already inserted a `:` so continue - // with a comma. - InsertPrefix = FirstToCtorInits ? " : " : ", "; - } - } + if (Init->isMemberInitializer() && + Index < Init->getMember()->getFieldIndex()) { + InsertPos = Init->getSourceLocation(); + // There are initializers after the one we are inserting, so add a + // comma after this insertion in order to not break anything. + AddComma = true; + break; + } + LastInListInit = Init; + } + if (HasInitAlready) { + if (InsertPos.isValid()) InvalidFix |= InsertPos.isMacroID(); + else + InvalidFix |= ReplaceRange.getBegin().isMacroID() || + ReplaceRange.getEnd().isMacroID(); + } else { + if (InsertPos.isInvalid()) { + if (LastInListInit) { + InsertPos = Lexer::getLocForEndOfToken( + LastInListInit->getRParenLoc(), 0, *Result.SourceManager, + getLangOpts()); + // Inserting after the last constructor initializer, so we need a + // comma. + InsertPrefix = ", "; + } else { + InsertPos = Lexer::getLocForEndOfToken( + Ctor->getTypeSourceInfo() + ->getTypeLoc() + .getAs() + .getLocalRangeEnd(), + 0, *Result.SourceManager, getLangOpts()); + + // If this is first time in the loop, there are no initializers so + // `:` declares member initialization list. If this is a + // subsequent pass then we have already inserted a `:` so continue + // with a comma. + InsertPrefix = FirstToCtorInits ? " : " : ", "; + } } + InvalidFix |= InsertPos.isMacroID(); + } - SourceLocation SemiColonEnd; - if (auto NextToken = Lexer::findNextToken( - S->getEndLoc(), *Result.SourceManager, getLangOpts())) - SemiColonEnd = NextToken->getEndLoc(); + SourceLocation SemiColonEnd; + if (auto NextToken = Lexer::findNextToken( + S->getEndLoc(), *Result.SourceManager, getLangOpts())) + SemiColonEnd = NextToken->getEndLoc(); + else + InvalidFix = true; + + auto Diag = diag(S->getBeginLoc(), "%0 should be initialized in a member" + " initializer of the constructor") + << Field; + if (InvalidFix) + continue; + StringRef NewInit = Lexer::getSourceText( + CharSourceRange(InitValue->getSourceRange(), true), + *Result.SourceManager, getLangOpts()); + if (HasInitAlready) { + if (InsertPos.isValid()) + Diag << FixItHint::CreateInsertion(InsertPos, NewInit); else - InvalidFix = true; - - auto Diag = - diag(S->getBeginLoc(), "%0 should be initialized in a member" - " initializer of the constructor") - << Field; - if (InvalidFix) - continue; - StringRef NewInit = Lexer::getSourceText( - CharSourceRange(InitValue->getSourceRange(), true), - *Result.SourceManager, getLangOpts()); - if (HasInitAlready) { - if (InsertPos.isValid()) - Diag << FixItHint::CreateInsertion(InsertPos, NewInit); - else - Diag << FixItHint::CreateReplacement(ReplaceRange, NewInit); - } else { - SmallString<128> Insertion({InsertPrefix, Field->getName(), "(", - NewInit, AddComma ? "), " : ")"}); - Diag << FixItHint::CreateInsertion(InsertPos, Insertion, - FirstToCtorInits); - FirstToCtorInits = areDiagsSelfContained(); - } - Diag << FixItHint::CreateRemoval( - CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd)); + Diag << FixItHint::CreateReplacement(ReplaceRange, NewInit); + } else { + SmallString<128> Insertion({InsertPrefix, Field->getName(), "(", + NewInit, AddComma ? "), " : ")"}); + Diag << FixItHint::CreateInsertion(InsertPos, Insertion, + FirstToCtorInits); + FirstToCtorInits = areDiagsSelfContained(); } + Diag << FixItHint::CreateRemoval( + CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd)); } } } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -174,6 +174,11 @@ ` to :doc:`bugprone-unsafe-functions ` was added. +- New alias :doc:`cppcoreguidelines-use-default-member-init + ` to + :doc:`modernize-use-default-member-init + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ - Improved :doc:`readability-redundant-string-cstr @@ -215,6 +220,11 @@ - Deprecated :doc:`cert-dcl21-cpp ` check. +- Move C.48 enforcement from :doc:`cppcoreguidelines-prefer-member-initializer + ` to + :doc:`cppcoreguidelines-use-default-member-init + `. + - Deprecated check-local options `HeaderFileExtensions` in :doc:`google-build-namespaces ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst @@ -11,18 +11,6 @@ This check implements `C.49 `_ from the CppCoreGuidelines. -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. - -This latter rule is `C.48 `_ from CppCoreGuidelines. - -Please note, that this check does not enforce this latter rule for -initializations already implemented as member initializers. For that purpose -see check `modernize-use-default-member-init <../modernize/use-default-member-init.html>`_. - Example 1 --------- @@ -40,16 +28,16 @@ } }; -Here ``n`` can be initialized using a default member initializer, unlike +Here ``n`` can be initialized in the member initializer list, unlike ``m``, as ``m``'s initialization follows a control statement (``if``): .. code-block:: c++ class C { - int n{1}; + int n; int m; public: - C() { + C(): n(1) { if (dice()) return; m = 1; @@ -82,22 +70,3 @@ return; m = mm; } - -.. option:: UseAssignment - - If this option is set to `true` (default is `false`), 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; - } - }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-default-member-init.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-default-member-init.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-default-member-init.rst @@ -0,0 +1,12 @@ +.. title:: clang-tidy - cppcoreguidelines-use-default-member-init +.. meta:: + :http-equiv=refresh: 5;URL=../modernize/use-default-member-init.html + +cppcoreguidelines-use-default-member-init +========================================= + +This check implements `C.48 `_ from the CppCoreGuidelines. + +The cppcoreguidelines-use-default-member-init check is an alias, please see +`modernize-use-default-member-init <../modernize/use-default-member-init.html>`_ +for more information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -207,6 +207,7 @@ `cppcoreguidelines-rvalue-reference-param-not-moved `_, `cppcoreguidelines-slicing `_, `cppcoreguidelines-special-member-functions `_, + `cppcoreguidelines-use-default-member-init `_, "Yes" `cppcoreguidelines-virtual-class-destructor `_, "Yes" `darwin-avoid-spinlock `_, `darwin-dispatch-once-nonstatic `_, "Yes" diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init-assignment.cpp deleted file mode 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init-assignment.cpp +++ /dev/null @@ -1,31 +0,0 @@ -// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \ -// RUN: -config="{CheckOptions: [{key: modernize-use-default-member-init.UseAssignment, value: true}]}" - -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; -}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init.cpp deleted file mode 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init.cpp +++ /dev/null @@ -1,30 +0,0 @@ -// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %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; -};