Skip to content

Commit cd477e7

Browse files
committed
[clangd] Fix modernize-loop-convert "multiple diag in flight" crash.
Summary: this maybe not ideal, but it is trivial and does fix the crash. Fixes clangd/clangd#156. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D78715 (cherry picked from commit a466e4b)
1 parent 85a2d23 commit cd477e7

File tree

2 files changed

+35
-9
lines changed

2 files changed

+35
-9
lines changed

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -522,13 +522,11 @@ void LoopConvertCheck::doConversion(
522522
const ValueDecl *MaybeContainer, const UsageResult &Usages,
523523
const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit,
524524
const ForStmt *Loop, RangeDescriptor Descriptor) {
525-
auto Diag = diag(Loop->getForLoc(), "use range-based for loop instead");
526-
527525
std::string VarName;
528526
bool VarNameFromAlias = (Usages.size() == 1) && AliasDecl;
529527
bool AliasVarIsRef = false;
530528
bool CanCopy = true;
531-
529+
std::vector<FixItHint> FixIts;
532530
if (VarNameFromAlias) {
533531
const auto *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl());
534532
VarName = AliasVar->getName().str();
@@ -560,8 +558,8 @@ void LoopConvertCheck::doConversion(
560558
getAliasRange(Context->getSourceManager(), ReplaceRange);
561559
}
562560

563-
Diag << FixItHint::CreateReplacement(
564-
CharSourceRange::getTokenRange(ReplaceRange), ReplacementText);
561+
FixIts.push_back(FixItHint::CreateReplacement(
562+
CharSourceRange::getTokenRange(ReplaceRange), ReplacementText));
565563
// No further replacements are made to the loop, since the iterator or index
566564
// was used exactly once - in the initialization of AliasVar.
567565
} else {
@@ -606,8 +604,8 @@ void LoopConvertCheck::doConversion(
606604
Usage.Kind == Usage::UK_CaptureByCopy ? "&" + VarName : VarName;
607605
}
608606
TUInfo->getReplacedVars().insert(std::make_pair(Loop, IndexVar));
609-
Diag << FixItHint::CreateReplacement(
610-
CharSourceRange::getTokenRange(Range), ReplaceText);
607+
FixIts.push_back(FixItHint::CreateReplacement(
608+
CharSourceRange::getTokenRange(Range), ReplaceText));
611609
}
612610
}
613611

@@ -645,8 +643,9 @@ void LoopConvertCheck::doConversion(
645643
std::string Range = ("(" + TypeString + " " + VarName + " : " +
646644
MaybeDereference + Descriptor.ContainerString + ")")
647645
.str();
648-
Diag << FixItHint::CreateReplacement(
649-
CharSourceRange::getTokenRange(ParenRange), Range);
646+
FixIts.push_back(FixItHint::CreateReplacement(
647+
CharSourceRange::getTokenRange(ParenRange), Range));
648+
diag(Loop->getForLoc(), "use range-based for loop instead") << FixIts;
650649
TUInfo->getGeneratedDecls().insert(make_pair(Loop, VarName));
651650
}
652651

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,33 @@ TEST(DiagnosticsTest, ClangTidy) {
266266
));
267267
}
268268

269+
TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
270+
Annotations Main(R"cpp(
271+
template <typename T> struct Foo {
272+
T *begin();
273+
T *end();
274+
};
275+
struct LabelInfo {
276+
int a;
277+
bool b;
278+
};
279+
280+
void f() {
281+
Foo<LabelInfo> label_info_map;
282+
[[for]] (auto it = label_info_map.begin(); it != label_info_map.end(); ++it) {
283+
auto S = *it;
284+
}
285+
}
286+
)cpp");
287+
TestTU TU = TestTU::withCode(Main.code());
288+
TU.ClangTidyChecks = "modernize-loop-convert";
289+
EXPECT_THAT(
290+
TU.build().getDiagnostics(),
291+
UnorderedElementsAre(::testing::AllOf(
292+
Diag(Main.range(), "use range-based for loop instead"),
293+
DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"))));
294+
}
295+
269296
TEST(DiagnosticTest, ClangTidySuppressionComment) {
270297
Annotations Main(R"cpp(
271298
int main() {

0 commit comments

Comments
 (0)