[clang-tidy] Remove enforcement of rule C.48 from cppcoreguidelines-prefer-member-init #80330
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
February 1, 2024 19:34
…refer-member-init This functionality already exists in cppcoreguidelines-use-default-member-init. It was deprecated from this check in clang-tidy 17. This allows us to fully decouple this check from the corresponding modernize check, which has an unhealthy dependency. Fixes llvm#62169
|
@llvm/pr-subscribers-clang-tidy Author: Carlos Galvez (carlosgalvezp) ChangesThis functionality already exists in This allows us to fully decouple this check from the corresponding modernize check, which has an unhealthy dependency. Fixes #62169 Patch is 20.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80330.diff 6 Files Affected:
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<StringLiteral, CharacterLiteral, IntegerLiteral, FloatingLiteral,
- CXXBoolLiteralExpr, 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;
-}
-
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<RecordDecl>(Class->getDeclContext()) ||
- !cast<RecordDecl>(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<ImplicitValueInitExpr>(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<ImplicitValueInitExpr>(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<clang::FunctionTypeLoc>()
- .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<clang::FunctionTypeLoc>()
+ .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
+ <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>`
+ by removing enforcement of rule `C.48
+ <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_,
+ which was deprecated since :program:`clang-tidy` 17. This rule is now covered
+ by :doc:`cppcoreguidelines-use-default-member-init
+ <clang-tidy/checks/cppcoreguidelines/use-default-member-init>`.
+
- Improved :doc:`modernize-avoid-c-arrays
<clang-tidy/checks/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
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c49-prefer-initialization-to-assignment-in-constructors>`_
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
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_
-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;
- ...
[truncated]
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Carlos Galvez (carlosgalvezp) ChangesThis functionality already exists in This allows us to fully decouple this check from the corresponding modernize check, which has an unhealthy dependency. Fixes #62169 Patch is 20.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80330.diff 6 Files Affected:
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<StringLiteral, CharacterLiteral, IntegerLiteral, FloatingLiteral,
- CXXBoolLiteralExpr, 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;
-}
-
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<RecordDecl>(Class->getDeclContext()) ||
- !cast<RecordDecl>(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<ImplicitValueInitExpr>(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<ImplicitValueInitExpr>(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<clang::FunctionTypeLoc>()
- .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<clang::FunctionTypeLoc>()
+ .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
+ <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>`
+ by removing enforcement of rule `C.48
+ <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_,
+ which was deprecated since :program:`clang-tidy` 17. This rule is now covered
+ by :doc:`cppcoreguidelines-use-default-member-init
+ <clang-tidy/checks/cppcoreguidelines/use-default-member-init>`.
+
- Improved :doc:`modernize-avoid-c-arrays
<clang-tidy/checks/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
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c49-prefer-initialization-to-assignment-in-constructors>`_
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
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_
-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;
- ...
[truncated]
|
PiotrZSL
approved these changes
Feb 1, 2024
ichaer
added a commit
to ichaer/llvm-project-onesided_lower_bound
that referenced
this pull request
Feb 2, 2024
* llvm/main: (500 commits) [docs] Add beginner-focused office hours (llvm#80308) [mlir][sparse] external entry method wrapper for sparse tensors (llvm#80326) [StackSlotColoring] Ignore non-spill objects in RemoveDeadStores. (llvm#80242) [libc][stdbit] fix return types (llvm#80337) Revert "[RISCV] Refine cost on Min/Max reduction" (llvm#80340) [TTI]Add support for strided loads/stores. [analyzer][HTMLRewriter] Cache partial rewrite results. (llvm#80220) [flang][openacc][openmp] Use #0 from hlfir.declare value when generating bound ops (llvm#80317) [AArch64][PAC] Expand blend(reg, imm) operation in aarch64-pauth pass (llvm#74729) [SHT_LLVM_BB_ADDR_MAP][llvm-readobj] Implements llvm-readobj handling for PGOAnalysisMap. (llvm#79520) [libc] add bazel support for most of unistd (llvm#80078) [clang-tidy] Remove enforcement of rule C.48 from cppcoreguidelines-prefer-member-init (llvm#80330) [OpenMP] Fix typo (NFC) (llvm#80332) [BOLT] Enable re-writing of Linux kernel binary (llvm#80228) [BOLT] Adjust section sizes based on file offsets (llvm#80226) [libc] fix stdbit include test when not all entrypoints are available (llvm#80323) [RISCV][GISel] RegBank select and instruction select for vector G_ADD, G_SUB (llvm#74114) [RISCV] Add srmcfg CSR from Ssqosid extension. (llvm#79914) [mlir][sparse] add sparsification options to pretty print and debug s… (llvm#80205) [RISCV][MC] MC layer support for the experimental zalasr extension (llvm#79911) ...
agozillon
pushed a commit
to agozillon/llvm-project
that referenced
this pull request
Feb 5, 2024
…refer-member-init (llvm#80330) This functionality already exists in cppcoreguidelines-use-default-member-init. It was deprecated from this check in clang-tidy 17. This allows us to fully decouple this check from the corresponding modernize check, which has an unhealthy dependency. Fixes llvm#62169 --------- Co-authored-by: Carlos Gálvez <carlos.galvez@zenseact.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This functionality already exists in
cppcoreguidelines-use-default-member-init. It was deprecated from this check in clang-tidy 17.
This allows us to fully decouple this check from the corresponding modernize check, which has an unhealthy dependency.
Fixes #62169