diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp index f79a67819bb85..de96c3dc4efef 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp @@ -34,27 +34,6 @@ static bool isNoReturnCallStatement(const Stmt *S) { return Func->isNoReturn(); } -static bool isLiteral(const Expr *E) { - return isa(E); -} - -static bool isUnaryExprOfLiteral(const Expr *E) { - if (const auto *UnOp = dyn_cast(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(Value)) - return isa(DRE->getDecl()); - - return false; -} - namespace { AST_MATCHER_P(FieldDecl, indexNotLessThan, unsigned, Index) { @@ -166,19 +145,7 @@ isAssignmentToMemberOf(const CXXRecordDecl *Rec, const Stmt *S, PreferMemberInitializerCheck::PreferMemberInitializerCheck( StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - IsUseDefaultMemberInitEnabled( - Context->isCheckEnabled("modernize-use-default-member-init")), - UseAssignment( - Options.get("UseAssignment", - OptionsView("modernize-use-default-member-init", - Context->getOptions().CheckOptions, Context) - .get("UseAssignment", false))) {} - -void PreferMemberInitializerCheck::storeOptions( - ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "UseAssignment", UseAssignment); -} + : ClangTidyCheck(Name, Context) {} void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(cxxConstructorDecl(hasBody(compoundStmt()), @@ -230,139 +197,99 @@ void PreferMemberInitializerCheck::check( updateAssignmentLevel(Field, InitValue, Ctor, AssignedFields); if (!canAdvanceAssignment(AssignedFields[Field])) continue; - const bool IsInDefaultMemberInitializer = - IsUseDefaultMemberInitEnabled && getLangOpts().CPlusPlus11 && - Ctor->isDefaultConstructor() && - (getLangOpts().CPlusPlus20 || !Field->isBitField()) && - !Field->hasInClassInitializer() && - (!isa(Class->getDeclContext()) || - !cast(Class->getDeclContext())->isUnion()) && - shouldBeDefaultMemberInitializer(InitValue); - if (IsInDefaultMemberInitializer) { - 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) - continue; - CharSourceRange StmtRange = - CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd); - SmallString<128> Insertion( - {UseAssignment ? " = " : "{", - Lexer::getSourceText(Result.SourceManager->getExpansionRange( - InitValue->getSourceRange()), - *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->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; + 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(); } - 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( + Result.SourceManager->getExpansionRange(InitValue->getSourceRange()), + *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( - Result.SourceManager->getExpansionRange(InitValue->getSourceRange()), - *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/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h index 7b39da1fd3c54..b3f8284b435af 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h @@ -24,12 +24,8 @@ class PreferMemberInitializerCheck : public ClangTidyCheck { bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - - const bool IsUseDefaultMemberInitEnabled; - const bool UseAssignment; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2a7749d71405c..d654e6c2b60b0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -106,6 +106,14 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Cleaned up :doc:`cppcoreguidelines-prefer-member-initializer + ` + by removing enforcement of rule `C.48 + `_, + which was deprecated since :program:`clang-tidy` 17. This rule is now covered + by :doc:`cppcoreguidelines-use-default-member-init + `. + - Improved :doc:`modernize-avoid-c-arrays ` check by introducing the new `AllowStringArrays` option, enabling the exclusion of array types with deduced 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 index 81aa662043bc3..6d1bdb93cc7b0 100644 --- 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 @@ -13,27 +13,11 @@ This check implements `C.49 `_ from the C++ Core Guidelines. -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 +Please note, that this check does not enforce rule `C.48 `_ -from the C++ Core Guidelines. - -Please note, that this check does not enforce this latter rule for -initializations already implemented as member initializers. For that purpose +from the C++ Core Guidelines. For that purpose see check :doc:`modernize-use-default-member-init <../modernize/use-default-member-init>`. -.. note:: - - Enforcement of rule C.48 in this check is deprecated, to be removed in - :program:`clang-tidy` version 19 (only C.49 will be enforced by this check then). - Please use :doc:`cppcoreguidelines-use-default-member-init <../cppcoreguidelines/use-default-member-init>` - to enforce rule C.48. - Example 1 --------- @@ -51,16 +35,16 @@ Example 1 } }; -Here ``n`` can be initialized using a default member initializer, unlike +Here ``n`` can be initialized in the constructor 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; @@ -84,7 +68,7 @@ Example 2 } }; -Here ``n`` can be initialized in the constructor initialization list, unlike +Here ``n`` can be initialized in the constructor initializer list, unlike ``m``, as ``m``'s initialization follows a control statement (``if``): .. code-block:: c++ @@ -94,29 +78,3 @@ Here ``n`` can be initialized in the constructor initialization list, unlike return; m = mm; } - -.. option:: UseAssignment - - Note: this option is deprecated, to be removed in :program:`clang-tidy` - version 19. Please use the `UseAssignment` option from - :doc:`cppcoreguidelines-use-default-member-init <../cppcoreguidelines/use-default-member-init>` - instead. - - If this option is set to `true` (by default `UseAssignment` from - :doc:`modernize-use-default-member-init - <../modernize/use-default-member-init>` will be used), - 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/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 index 70ef19181f7ad..0000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init-assignment.cpp +++ /dev/null @@ -1,33 +0,0 @@ -// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \ -// RUN: -config="{CheckOptions: {modernize-use-default-member-init.UseAssignment: true}}" -// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \ -// RUN: -config="{CheckOptions: {modernize-use-default-member-init.UseAssignment: false, cppcoreguidelines-prefer-member-initializer.UseAssignment: 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 index 281a817a42b30..0000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init.cpp +++ /dev/null @@ -1,33 +0,0 @@ -// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer,modernize-use-default-member-init %t -- \ -// RUN: -config="{CheckOptions: {modernize-use-default-member-init.UseAssignment: true, \ -// RUN: cppcoreguidelines-prefer-member-initializer.UseAssignment: false}}" - -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; -};