[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0fcbb37838a: [clang-tidy] Fix invalid fix-it for
cppcoreguidelines-prefer-member-initializer (authored by njames93).
Herald added a project: All.
Changed prior to commit:
https://reviews.llvm.org/D118927?vs=410187=421292#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118927/new/
https://reviews.llvm.org/D118927
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -526,14 +526,32 @@
}
};
-struct AlreadyHasInit {
+struct HasInClassInit {
int m = 4;
- AlreadyHasInit() {
+ HasInClassInit() {
m = 3;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'm' should be initialized in a member initializer of the constructor
}
};
+struct HasInitListInit {
+ int M;
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: HasInitListInit(const HasInitListInit ) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ HasInitListInit(const HasInitListInit ) : M(4) {
+M = Other.M;
+ }
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: HasInitListInit(HasInitListInit &) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ HasInitListInit(HasInitListInit &) : M() {
+M = Other.M;
+ }
+};
+
#define ASSIGN_IN_MACRO(FIELD, VALUE) FIELD = (VALUE);
struct MacroCantFix {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -139,6 +139,12 @@
- Fixed a crash in :doc:`bugprone-sizeof-expression ` when
`sizeof(...)` is compared agains a `__int128_t`.
+
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+ ` check.
+
+ Fixed an issue when there was already an initializer in the constructor and
+ the check would try to create another initializer for the same member.
Removed checks
^^
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
@@ -201,30 +201,42 @@
diag(S->getBeginLoc(), "%0 should be initialized in an in-class"
" default member initializer")
<< Field;
-if (!InvalidFix) {
- 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);
-}
+if (InvalidFix)
+ 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())
+ if (!Init->isWritten() || Init->isInClassMemberInitializer())
continue;
+ if (Init->getMember() == Field) {
+HasInitAlready = true;
+if
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
njames93 updated this revision to Diff 410187.
njames93 marked 2 inline comments as done.
njames93 added a comment.
Remove double negative
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118927/new/
https://reviews.llvm.org/D118927
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -526,14 +526,32 @@
}
};
-struct AlreadyHasInit {
+struct HasInClassInit {
int m = 4;
- AlreadyHasInit() {
+ HasInClassInit() {
m = 3;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'm' should be initialized in a member initializer of the constructor
}
};
+struct HasInitListInit {
+ int M;
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: HasInitListInit(const HasInitListInit ) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ HasInitListInit(const HasInitListInit ) : M(4) {
+M = Other.M;
+ }
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: HasInitListInit(HasInitListInit &) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ HasInitListInit(HasInitListInit &) : M() {
+M = Other.M;
+ }
+};
+
#define ASSIGN_IN_MACRO(FIELD, VALUE) FIELD = (VALUE);
struct MacroCantFix {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -109,6 +109,12 @@
Changes in existing checks
^^
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+ ` check.
+
+ Fixed an issue when there was already an initializer in the constructor and
+ the check would try to create another initializer for the same member.
+
Removed checks
^^
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
@@ -201,30 +201,42 @@
diag(S->getBeginLoc(), "%0 should be initialized in an in-class"
" default member initializer")
<< Field;
-if (!InvalidFix) {
- 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);
-}
+if (InvalidFix)
+ 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())
+ 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()
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
LegalizeAdulthood accepted this revision. LegalizeAdulthood added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118927/new/ https://reviews.llvm.org/D118927 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
LegalizeAdulthood added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:251
+ if (InsertPos.isValid())
+InvalidFix |= InsertPos.isMacroID();
+ else
I'm not a fan of using bit-wise operators on booleans, but I see that
the code for this check was already doing that.
(See https://github.com/llvm/llvm-project/issues/40307)
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:293
<< Field;
if (!InvalidFix) {
+ StringRef NewInit = Lexer::getSourceText(
I'm not a fan of double negatives either `:)`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118927/new/
https://reviews.llvm.org/D118927
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
njames93 marked 2 inline comments as done.
njames93 added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp:550
+ // CHECK-FIXES-NEXT: }
+ PR53515(PR53515 &) : M() {
+M = Other.M;
JonasToth wrote:
> could you please add a test-case for initializer lists, like `std::vector`?
> Or is this not supported?
That's not what this fix is about.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118927/new/
https://reviews.llvm.org/D118927
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
njames93 updated this revision to Diff 405919.
njames93 added a comment.
Rename test case
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118927/new/
https://reviews.llvm.org/D118927
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -526,14 +526,32 @@
}
};
-struct AlreadyHasInit {
+struct HasInClassInit {
int m = 4;
- AlreadyHasInit() {
+ HasInClassInit() {
m = 3;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'm' should be initialized in a member initializer of the constructor
}
};
+struct HasInitListInit {
+ int M;
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: HasInitListInit(const HasInitListInit ) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ HasInitListInit(const HasInitListInit ) : M(4) {
+M = Other.M;
+ }
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: HasInitListInit(HasInitListInit &) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ HasInitListInit(HasInitListInit &) : M() {
+M = Other.M;
+ }
+};
+
#define ASSIGN_IN_MACRO(FIELD, VALUE) FIELD = (VALUE);
struct MacroCantFix {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -105,6 +105,12 @@
Changes in existing checks
^^
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+ ` check.
+
+ Fixed an issue when there was already an initializer in the constructor and
+ the check would try to create another initializer for the same member.
+
Removed checks
^^
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
@@ -217,14 +217,25 @@
}
} 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())
+ 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();
@@ -235,30 +246,38 @@
}
LastInListInit = Init;
}
-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 (HasInitAlready) {
+ if (InsertPos.isValid())
+InvalidFix |= InsertPos.isMacroID();
+ else
+InvalidFix |=
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
JonasToth added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp:550
+ // CHECK-FIXES-NEXT: }
+ PR53515(PR53515 &) : M() {
+M = Other.M;
could you please add a test-case for initializer lists, like `std::vector`? Or
is this not supported?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118927/new/
https://reviews.llvm.org/D118927
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
simon.giesecke added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp:537
+struct PR53515 {
+ int M;
Can this be renamed to something describing the test case? E.g.
`AlreadyHasInitializer` (and the case above might be renamed to
`AlreadyHasDefaultInitializer` to differentiate the two cases)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118927/new/
https://reviews.llvm.org/D118927
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer
njames93 created this revision.
njames93 added reviewers: LegalizeAdulthood, aaron.ballman, alexfh.
Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, nemanjai.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Fixes https://github.com/llvm/llvm-project/issues/53515.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D118927
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
@@ -534,6 +534,24 @@
}
};
+struct PR53515 {
+ int M;
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: PR53515(const PR53515 ) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ PR53515(const PR53515 ) : M(4) {
+M = Other.M;
+ }
+ // CHECK-MESSAGES: :[[@LINE+5]]:5: warning: 'M' should be initialized in a member initializer of the constructor
+ // CHECK-FIXES: PR53515(PR53515 &) : M(Other.M) {
+ // CHECK-FIXES-NEXT: {{^$}}
+ // CHECK-FIXES-NEXT: }
+ PR53515(PR53515 &) : M() {
+M = Other.M;
+ }
+};
+
#define ASSIGN_IN_MACRO(FIELD, VALUE) FIELD = (VALUE);
struct MacroCantFix {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -105,6 +105,12 @@
Changes in existing checks
^^
+- Improved :doc:`cppcoreguidelines-prefer-member-initializer
+ ` check.
+
+ Fixed an issue when there was already an initializer in the constructor and
+ the check would try to create another initializer for the same member.
+
Removed checks
^^
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
@@ -217,14 +217,25 @@
}
} 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())
+ 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();
@@ -235,30 +246,38 @@
}
LastInListInit = Init;
}
-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 (HasInitAlready) {
+ if (InsertPos.isValid())
+InvalidFix |= InsertPos.isMacroID();
+ else
+InvalidFix |= ReplaceRange.getBegin().isMacroID() ||
+ ReplaceRange.getEnd().isMacroID();
+} else {
+ if
