hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
this maybe not ideal, but it is trivial and does fix the crash.
Fixes https://github.com/clangd/clangd/issues/156.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78715
Files:
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -272,6 +272,33 @@
"use a trailing return type for this function")))));
}
+TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
+ Annotations Main(R"cpp(
+ template <typename T> struct Foo {
+ T *begin();
+ T *end();
+ };
+ struct LabelInfo {
+ int a;
+ bool b;
+ };
+
+ void f() {
+ Foo<LabelInfo> label_info_map;
+ [[for]] (auto it = label_info_map.begin(); it != label_info_map.end();
++it) {
+ auto S = *it;
+ }
+ }
+ )cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.ClangTidyChecks = "modernize-loop-convert";
+ EXPECT_THAT(
+ TU.build().getDiagnostics(),
+ UnorderedElementsAre(::testing::AllOf(
+ Diag(Main.range(), "use range-based for loop instead"),
+ DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"))));
+}
+
TEST(DiagnosticTest, ClangTidySuppressionComment) {
Annotations Main(R"cpp(
int main() {
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -525,6 +525,9 @@
const ValueDecl *MaybeContainer, const UsageResult &Usages,
const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit,
const ForStmt *Loop, RangeDescriptor Descriptor) {
+ // getTypeInfo may emit a diagnostic, we have to call it before constructing
+ // the Diag to avoid the "multiple-diagnostic in flight" crash in clangd.
+ auto TypeWidth = Context->getTypeInfo(Descriptor.ElemType).Width;
auto Diag = diag(Loop->getForLoc(), "use range-based for loop instead");
std::string VarName;
@@ -628,7 +631,7 @@
!Descriptor.ElemType.isNull() &&
Descriptor.ElemType.isTriviallyCopyableType(*Context) &&
// TypeInfo::Width is in bits.
- Context->getTypeInfo(Descriptor.ElemType).Width <= 8 * MaxCopySize;
+ TypeWidth <= 8 * MaxCopySize;
bool UseCopy = CanCopy && ((VarNameFromAlias && !AliasVarIsRef) ||
(Descriptor.DerefByConstRef && IsCheapToCopy));
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -272,6 +272,33 @@
"use a trailing return type for this function")))));
}
+TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
+ Annotations Main(R"cpp(
+ template <typename T> struct Foo {
+ T *begin();
+ T *end();
+ };
+ struct LabelInfo {
+ int a;
+ bool b;
+ };
+
+ void f() {
+ Foo<LabelInfo> label_info_map;
+ [[for]] (auto it = label_info_map.begin(); it != label_info_map.end(); ++it) {
+ auto S = *it;
+ }
+ }
+ )cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.ClangTidyChecks = "modernize-loop-convert";
+ EXPECT_THAT(
+ TU.build().getDiagnostics(),
+ UnorderedElementsAre(::testing::AllOf(
+ Diag(Main.range(), "use range-based for loop instead"),
+ DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"))));
+}
+
TEST(DiagnosticTest, ClangTidySuppressionComment) {
Annotations Main(R"cpp(
int main() {
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -525,6 +525,9 @@
const ValueDecl *MaybeContainer, const UsageResult &Usages,
const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit,
const ForStmt *Loop, RangeDescriptor Descriptor) {
+ // getTypeInfo may emit a diagnostic, we have to call it before constructing
+ // the Diag to avoid the "multiple-diagnostic in flight" crash in clangd.
+ auto TypeWidth = Context->getTypeInfo(Descriptor.ElemType).Width;
auto Diag = diag(Loop->getForLoc(), "use range-based for loop instead");
std::string VarName;
@@ -628,7 +631,7 @@
!Descriptor.ElemType.isNull() &&
Descriptor.ElemType.isTriviallyCopyableType(*Context) &&
// TypeInfo::Width is in bits.
- Context->getTypeInfo(Descriptor.ElemType).Width <= 8 * MaxCopySize;
+ TypeWidth <= 8 * MaxCopySize;
bool UseCopy = CanCopy && ((VarNameFromAlias && !AliasVarIsRef) ||
(Descriptor.DerefByConstRef && IsCheapToCopy));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
