carlosgalvezp created this revision.
Herald added subscribers: PiotrZSL, jeroen.dobbelaere, shchenz, kbarton,
xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
And remove identical functionality from
cppcoreguidelines-prefer-member-initializer, which had too many
responsibilities and a tight coupling to the
modernize-use-default-member-init check.
Fixes https://github.com/llvm/llvm-project/issues/62164.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148460
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
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/cppcoreguidelines/use-default-member-init.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init.cpp
===================================================================
--- 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;
-};
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
===================================================================
--- 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;
-};
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
@@ -207,6 +207,7 @@
`cppcoreguidelines-rvalue-reference-param-not-moved <cppcoreguidelines/rvalue-reference-param-not-moved.html>`_,
`cppcoreguidelines-slicing <cppcoreguidelines/slicing.html>`_,
`cppcoreguidelines-special-member-functions <cppcoreguidelines/special-member-functions.html>`_,
+ `cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init.html>`_, "Yes"
`cppcoreguidelines-virtual-class-destructor <cppcoreguidelines/virtual-class-destructor.html>`_, "Yes"
`darwin-avoid-spinlock <darwin/avoid-spinlock.html>`_,
`darwin-dispatch-once-nonstatic <darwin/dispatch-once-nonstatic.html>`_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/use-default-member-init.rst
===================================================================
--- /dev/null
+++ 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 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_ 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.
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst
@@ -11,18 +11,6 @@
This check implements `C.49 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c49-prefer-initialization-to-assignment-in-constructors>`_ 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 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_ 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;
- }
- };
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -174,6 +174,11 @@
<clang-tidy/checks/cert/msc33-c>` to :doc:`bugprone-unsafe-functions
<clang-tidy/checks/bugprone/unsafe-functions>` was added.
+- New alias :doc:`cppcoreguidelines-use-default-member-init
+ <clang-tidy/checks/cppcoreguidelines/use-default-member-init>` to
+ :doc:`modernize-use-default-member-init
+ <clang-tidy/checks/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
<clang-tidy/checks/cert/dcl21-cpp>` check.
+- Move C.48 enforcement from :doc:`cppcoreguidelines-prefer-member-initializer
+ <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` to
+ :doc:`cppcoreguidelines-use-default-member-init
+ <clang-tidy/checks/cppcoreguidelines/use-default-member-init>`.
+
- Deprecated check-local options `HeaderFileExtensions`
in :doc:`google-build-namespaces
<clang-tidy/checks/google/build-namespaces>` check.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
+++ 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<RecordDecl>(Class->getDeclContext()) ||
- !cast<RecordDecl>(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<ImplicitValueInitExpr>(Init->getInit()))
- InsertPos = Init->getRParenLoc();
- else {
- ReplaceRange = Init->getInit()->getSourceRange();
- }
- break;
+ if (Init->getMember() == Field) {
+ HasInitAlready = true;
+ if (isa<ImplicitValueInitExpr>(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<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(
+ 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));
}
}
}
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
@@ -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<SpecialMemberFunctionsCheck>(
"cppcoreguidelines-special-member-functions");
CheckFactories.registerCheck<SlicingCheck>("cppcoreguidelines-slicing");
+ CheckFactories.registerCheck<modernize::UseDefaultMemberInitCheck>(
+ "cppcoreguidelines-use-default-member-init");
CheckFactories.registerCheck<misc::UnconventionalAssignOperatorCheck>(
"cppcoreguidelines-c-copy-assignment-signature");
CheckFactories.registerCheck<VirtualClassDestructorCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
