Skip to content

Commit 6f32d6a

Browse files
carlosgalvezpCarlos Gálvez
carlosgalvezp
and
Carlos Gálvez
authored
[clang-tidy] Remove enforcement of rule C.48 from cppcoreguidelines-prefer-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 <[email protected]>
1 parent a063df2 commit 6f32d6a

File tree

6 files changed

+100
-277
lines changed

6 files changed

+100
-277
lines changed

‎clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp‎

Lines changed: 86 additions & 159 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,6 @@ static bool isNoReturnCallStatement(const Stmt *S) {
3434
return Func->isNoReturn();
3535
}
3636

37-
static bool isLiteral(const Expr *E) {
38-
return isa<StringLiteral, CharacterLiteral, IntegerLiteral, FloatingLiteral,
39-
CXXBoolLiteralExpr, CXXNullPtrLiteralExpr>(E);
40-
}
41-
42-
static bool isUnaryExprOfLiteral(const Expr *E) {
43-
if (const auto *UnOp = dyn_cast<UnaryOperator>(E))
44-
return isLiteral(UnOp->getSubExpr());
45-
return false;
46-
}
47-
48-
static bool shouldBeDefaultMemberInitializer(const Expr *Value) {
49-
if (isLiteral(Value) || isUnaryExprOfLiteral(Value))
50-
return true;
51-
52-
if (const auto *DRE = dyn_cast<DeclRefExpr>(Value))
53-
return isa<EnumConstantDecl>(DRE->getDecl());
54-
55-
return false;
56-
}
57-
5837
namespace {
5938

6039
AST_MATCHER_P(FieldDecl, indexNotLessThan, unsigned, Index) {
@@ -166,19 +145,7 @@ isAssignmentToMemberOf(const CXXRecordDecl *Rec, const Stmt *S,
166145

167146
PreferMemberInitializerCheck::PreferMemberInitializerCheck(
168147
StringRef Name, ClangTidyContext *Context)
169-
: ClangTidyCheck(Name, Context),
170-
IsUseDefaultMemberInitEnabled(
171-
Context->isCheckEnabled("modernize-use-default-member-init")),
172-
UseAssignment(
173-
Options.get("UseAssignment",
174-
OptionsView("modernize-use-default-member-init",
175-
Context->getOptions().CheckOptions, Context)
176-
.get("UseAssignment", false))) {}
177-
178-
void PreferMemberInitializerCheck::storeOptions(
179-
ClangTidyOptions::OptionMap &Opts) {
180-
Options.store(Opts, "UseAssignment", UseAssignment);
181-
}
148+
: ClangTidyCheck(Name, Context) {}
182149

183150
void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) {
184151
Finder->addMatcher(cxxConstructorDecl(hasBody(compoundStmt()),
@@ -230,139 +197,99 @@ void PreferMemberInitializerCheck::check(
230197
updateAssignmentLevel(Field, InitValue, Ctor, AssignedFields);
231198
if (!canAdvanceAssignment(AssignedFields[Field]))
232199
continue;
233-
const bool IsInDefaultMemberInitializer =
234-
IsUseDefaultMemberInitEnabled && getLangOpts().CPlusPlus11 &&
235-
Ctor->isDefaultConstructor() &&
236-
(getLangOpts().CPlusPlus20 || !Field->isBitField()) &&
237-
!Field->hasInClassInitializer() &&
238-
(!isa<RecordDecl>(Class->getDeclContext()) ||
239-
!cast<RecordDecl>(Class->getDeclContext())->isUnion()) &&
240-
shouldBeDefaultMemberInitializer(InitValue);
241-
if (IsInDefaultMemberInitializer) {
242-
bool InvalidFix = false;
243-
SourceLocation FieldEnd =
244-
Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0,
245-
*Result.SourceManager, getLangOpts());
246-
InvalidFix |= FieldEnd.isInvalid() || FieldEnd.isMacroID();
247-
SourceLocation SemiColonEnd;
248-
if (auto NextToken = Lexer::findNextToken(
249-
S->getEndLoc(), *Result.SourceManager, getLangOpts()))
250-
SemiColonEnd = NextToken->getEndLoc();
251-
else
252-
InvalidFix = true;
253-
auto Diag =
254-
diag(S->getBeginLoc(), "%0 should be initialized in an in-class"
255-
" default member initializer")
256-
<< Field;
257-
if (InvalidFix)
258-
continue;
259-
CharSourceRange StmtRange =
260-
CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd);
261200

262-
SmallString<128> Insertion(
263-
{UseAssignment ? " = " : "{",
264-
Lexer::getSourceText(Result.SourceManager->getExpansionRange(
265-
InitValue->getSourceRange()),
266-
*Result.SourceManager, getLangOpts()),
267-
UseAssignment ? "" : "}"});
268-
269-
Diag << FixItHint::CreateInsertion(FieldEnd, Insertion)
270-
<< FixItHint::CreateRemoval(StmtRange);
271-
272-
} else {
273-
StringRef InsertPrefix = "";
274-
bool HasInitAlready = false;
275-
SourceLocation InsertPos;
276-
SourceRange ReplaceRange;
277-
bool AddComma = false;
278-
bool InvalidFix = false;
279-
unsigned Index = Field->getFieldIndex();
280-
const CXXCtorInitializer *LastInListInit = nullptr;
281-
for (const CXXCtorInitializer *Init : Ctor->inits()) {
282-
if (!Init->isWritten() || Init->isInClassMemberInitializer())
283-
continue;
284-
if (Init->getMember() == Field) {
285-
HasInitAlready = true;
286-
if (isa<ImplicitValueInitExpr>(Init->getInit()))
287-
InsertPos = Init->getRParenLoc();
288-
else {
289-
ReplaceRange = Init->getInit()->getSourceRange();
290-
}
291-
break;
292-
}
293-
if (Init->isMemberInitializer() &&
294-
Index < Init->getMember()->getFieldIndex()) {
295-
InsertPos = Init->getSourceLocation();
296-
// There are initializers after the one we are inserting, so add a
297-
// comma after this insertion in order to not break anything.
298-
AddComma = true;
299-
break;
201+
StringRef InsertPrefix = "";
202+
bool HasInitAlready = false;
203+
SourceLocation InsertPos;
204+
SourceRange ReplaceRange;
205+
bool AddComma = false;
206+
bool InvalidFix = false;
207+
unsigned Index = Field->getFieldIndex();
208+
const CXXCtorInitializer *LastInListInit = nullptr;
209+
for (const CXXCtorInitializer *Init : Ctor->inits()) {
210+
if (!Init->isWritten() || Init->isInClassMemberInitializer())
211+
continue;
212+
if (Init->getMember() == Field) {
213+
HasInitAlready = true;
214+
if (isa<ImplicitValueInitExpr>(Init->getInit()))
215+
InsertPos = Init->getRParenLoc();
216+
else {
217+
ReplaceRange = Init->getInit()->getSourceRange();
300218
}
301-
LastInListInit = Init;
219+
break;
302220
}
303-
if (HasInitAlready) {
304-
if (InsertPos.isValid())
305-
InvalidFix |= InsertPos.isMacroID();
306-
else
307-
InvalidFix |= ReplaceRange.getBegin().isMacroID() ||
308-
ReplaceRange.getEnd().isMacroID();
309-
} else {
310-
if (InsertPos.isInvalid()) {
311-
if (LastInListInit) {
312-
InsertPos = Lexer::getLocForEndOfToken(
313-
LastInListInit->getRParenLoc(), 0, *Result.SourceManager,
314-
getLangOpts());
315-
// Inserting after the last constructor initializer, so we need a
316-
// comma.
317-
InsertPrefix = ", ";
318-
} else {
319-
InsertPos = Lexer::getLocForEndOfToken(
320-
Ctor->getTypeSourceInfo()
321-
->getTypeLoc()
322-
.getAs<clang::FunctionTypeLoc>()
323-
.getLocalRangeEnd(),
324-
0, *Result.SourceManager, getLangOpts());
325-
326-
// If this is first time in the loop, there are no initializers so
327-
// `:` declares member initialization list. If this is a
328-
// subsequent pass then we have already inserted a `:` so continue
329-
// with a comma.
330-
InsertPrefix = FirstToCtorInits ? " : " : ", ";
331-
}
332-
}
221+
if (Init->isMemberInitializer() &&
222+
Index < Init->getMember()->getFieldIndex()) {
223+
InsertPos = Init->getSourceLocation();
224+
// There are initializers after the one we are inserting, so add a
225+
// comma after this insertion in order to not break anything.
226+
AddComma = true;
227+
break;
228+
}
229+
LastInListInit = Init;
230+
}
231+
if (HasInitAlready) {
232+
if (InsertPos.isValid())
333233
InvalidFix |= InsertPos.isMacroID();
234+
else
235+
InvalidFix |= ReplaceRange.getBegin().isMacroID() ||
236+
ReplaceRange.getEnd().isMacroID();
237+
} else {
238+
if (InsertPos.isInvalid()) {
239+
if (LastInListInit) {
240+
InsertPos =
241+
Lexer::getLocForEndOfToken(LastInListInit->getRParenLoc(), 0,
242+
*Result.SourceManager, getLangOpts());
243+
// Inserting after the last constructor initializer, so we need a
244+
// comma.
245+
InsertPrefix = ", ";
246+
} else {
247+
InsertPos = Lexer::getLocForEndOfToken(
248+
Ctor->getTypeSourceInfo()
249+
->getTypeLoc()
250+
.getAs<clang::FunctionTypeLoc>()
251+
.getLocalRangeEnd(),
252+
0, *Result.SourceManager, getLangOpts());
253+
254+
// If this is first time in the loop, there are no initializers so
255+
// `:` declares member initialization list. If this is a
256+
// subsequent pass then we have already inserted a `:` so continue
257+
// with a comma.
258+
InsertPrefix = FirstToCtorInits ? " : " : ", ";
259+
}
334260
}
261+
InvalidFix |= InsertPos.isMacroID();
262+
}
335263

336-
SourceLocation SemiColonEnd;
337-
if (auto NextToken = Lexer::findNextToken(
338-
S->getEndLoc(), *Result.SourceManager, getLangOpts()))
339-
SemiColonEnd = NextToken->getEndLoc();
264+
SourceLocation SemiColonEnd;
265+
if (auto NextToken = Lexer::findNextToken(
266+
S->getEndLoc(), *Result.SourceManager, getLangOpts()))
267+
SemiColonEnd = NextToken->getEndLoc();
268+
else
269+
InvalidFix = true;
270+
271+
auto Diag = diag(S->getBeginLoc(), "%0 should be initialized in a member"
272+
" initializer of the constructor")
273+
<< Field;
274+
if (InvalidFix)
275+
continue;
276+
StringRef NewInit = Lexer::getSourceText(
277+
Result.SourceManager->getExpansionRange(InitValue->getSourceRange()),
278+
*Result.SourceManager, getLangOpts());
279+
if (HasInitAlready) {
280+
if (InsertPos.isValid())
281+
Diag << FixItHint::CreateInsertion(InsertPos, NewInit);
340282
else
341-
InvalidFix = true;
342-
343-
auto Diag = diag(S->getBeginLoc(), "%0 should be initialized in a member"
344-
" initializer of the constructor")
345-
<< Field;
346-
if (InvalidFix)
347-
continue;
348-
StringRef NewInit = Lexer::getSourceText(
349-
Result.SourceManager->getExpansionRange(InitValue->getSourceRange()),
350-
*Result.SourceManager, getLangOpts());
351-
if (HasInitAlready) {
352-
if (InsertPos.isValid())
353-
Diag << FixItHint::CreateInsertion(InsertPos, NewInit);
354-
else
355-
Diag << FixItHint::CreateReplacement(ReplaceRange, NewInit);
356-
} else {
357-
SmallString<128> Insertion({InsertPrefix, Field->getName(), "(",
358-
NewInit, AddComma ? "), " : ")"});
359-
Diag << FixItHint::CreateInsertion(InsertPos, Insertion,
360-
FirstToCtorInits);
361-
FirstToCtorInits = areDiagsSelfContained();
362-
}
363-
Diag << FixItHint::CreateRemoval(
364-
CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd));
283+
Diag << FixItHint::CreateReplacement(ReplaceRange, NewInit);
284+
} else {
285+
SmallString<128> Insertion({InsertPrefix, Field->getName(), "(", NewInit,
286+
AddComma ? "), " : ")"});
287+
Diag << FixItHint::CreateInsertion(InsertPos, Insertion,
288+
FirstToCtorInits);
289+
FirstToCtorInits = areDiagsSelfContained();
365290
}
291+
Diag << FixItHint::CreateRemoval(
292+
CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd));
366293
}
367294
}
368295

‎clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,8 @@ class PreferMemberInitializerCheck : public ClangTidyCheck {
2424
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
2525
return LangOpts.CPlusPlus;
2626
}
27-
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
2827
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
2928
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
30-
31-
const bool IsUseDefaultMemberInitEnabled;
32-
const bool UseAssignment;
3329
};
3430

3531
} // namespace clang::tidy::cppcoreguidelines

‎clang-tools-extra/docs/ReleaseNotes.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ New check aliases
106106
Changes in existing checks
107107
^^^^^^^^^^^^^^^^^^^^^^^^^^
108108

109+
- Cleaned up :doc:`cppcoreguidelines-prefer-member-initializer
110+
<clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>`
111+
by removing enforcement of rule `C.48
112+
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_,
113+
which was deprecated since :program:`clang-tidy` 17. This rule is now covered
114+
by :doc:`cppcoreguidelines-use-default-member-init
115+
<clang-tidy/checks/cppcoreguidelines/use-default-member-init>`.
116+
109117
- Improved :doc:`modernize-avoid-c-arrays
110118
<clang-tidy/checks/modernize/avoid-c-arrays>` check by introducing the new
111119
`AllowStringArrays` option, enabling the exclusion of array types with deduced

‎clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/prefer-member-initializer.rst‎

Lines changed: 6 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,11 @@ This check implements `C.49
1313
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c49-prefer-initialization-to-assignment-in-constructors>`_
1414
from the C++ Core Guidelines.
1515

16-
If the language version is `C++ 11` or above, the constructor is the default
17-
constructor of the class, the field is not a bitfield (only in case of earlier
18-
language version than `C++ 20`), furthermore the assigned value is a literal,
19-
negated literal or ``enum`` constant then the preferred place of the
20-
initialization is at the class member declaration.
21-
22-
This latter rule is `C.48
16+
Please note, that this check does not enforce rule `C.48
2317
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c48-prefer-in-class-initializers-to-member-initializers-in-constructors-for-constant-initializers>`_
24-
from the C++ Core Guidelines.
25-
26-
Please note, that this check does not enforce this latter rule for
27-
initializations already implemented as member initializers. For that purpose
18+
from the C++ Core Guidelines. For that purpose
2819
see check :doc:`modernize-use-default-member-init <../modernize/use-default-member-init>`.
2920

30-
.. note::
31-
32-
Enforcement of rule C.48 in this check is deprecated, to be removed in
33-
:program:`clang-tidy` version 19 (only C.49 will be enforced by this check then).
34-
Please use :doc:`cppcoreguidelines-use-default-member-init <../cppcoreguidelines/use-default-member-init>`
35-
to enforce rule C.48.
36-
3721
Example 1
3822
---------
3923

@@ -51,16 +35,16 @@ Example 1
5135
}
5236
};
5337

54-
Here ``n`` can be initialized using a default member initializer, unlike
38+
Here ``n`` can be initialized in the constructor initializer list, unlike
5539
``m``, as ``m``'s initialization follows a control statement (``if``):
5640

5741
.. code-block:: c++
5842

5943
class C {
60-
int n{1};
44+
int n;
6145
int m;
6246
public:
63-
C() {
47+
C(): n(1) {
6448
if (dice())
6549
return;
6650
m = 1;
@@ -84,7 +68,7 @@ Example 2
8468
}
8569
};
8670

87-
Here ``n`` can be initialized in the constructor initialization list, unlike
71+
Here ``n`` can be initialized in the constructor initializer list, unlike
8872
``m``, as ``m``'s initialization follows a control statement (``if``):
8973

9074
.. code-block:: c++
@@ -94,29 +78,3 @@ Here ``n`` can be initialized in the constructor initialization list, unlike
9478
return;
9579
m = mm;
9680
}
97-
98-
.. option:: UseAssignment
99-
100-
Note: this option is deprecated, to be removed in :program:`clang-tidy`
101-
version 19. Please use the `UseAssignment` option from
102-
:doc:`cppcoreguidelines-use-default-member-init <../cppcoreguidelines/use-default-member-init>`
103-
instead.
104-
105-
If this option is set to `true` (by default `UseAssignment` from
106-
:doc:`modernize-use-default-member-init
107-
<../modernize/use-default-member-init>` will be used),
108-
the check will initialize members with an assignment.
109-
In this case the fix of the first example looks like this:
110-
111-
.. code-block:: c++
112-
113-
class C {
114-
int n = 1;
115-
int m;
116-
public:
117-
C() {
118-
if (dice())
119-
return;
120-
m = 1;
121-
}
122-
};

0 commit comments

Comments
 (0)