This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG08f0725a3a54: [clangd] Deduplicate missing-include findings
(authored by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149165/new/
https://reviews.llvm.org/D149165
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,36 @@
ParsedAST AST = TU.build();
auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
- // FIXME: Deduplicate references resulting from expansion of the same macro
in
- // multiple places.
- EXPECT_THAT(Findings, testing::SizeIs(2));
+ EXPECT_THAT(Findings, testing::SizeIs(1));
auto RefRange = Findings.front().SymRefRange;
auto &SM = AST.getSourceManager();
EXPECT_EQ(RefRange.file(), SM.getMainFileID());
// FIXME: Point at the spelling location, rather than the include.
EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
- EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+ Annotations MainFile(R"cpp(
+ #include "all.h"
+ FOO([[Foo]]);
+ )cpp");
+
+ TestTU TU;
+ TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+ TU.AdditionalFiles["all.h"] = guard(R"cpp(
+ #include "foo.h"
+ #define FOO(X) X y; X z
+ )cpp");
+
+ TU.Code = MainFile.code();
+ ParsedAST AST = TU.build();
+
+ auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+ EXPECT_THAT(Findings, testing::SizeIs(1));
+ auto RefRange = Findings.front().SymRefRange;
+ auto &SM = AST.getSourceManager();
+ EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+ EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
}
TEST(IncludeCleaner, NoCrash) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -40,6 +40,7 @@
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/GenericUniformityImpl.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallString.h"
@@ -415,6 +416,20 @@
Ref.Target, TouchingTokens.back().range(SM), Providers};
MissingIncludes.push_back(std::move(DiagInfo));
});
+ // Put possibly equal diagnostics together for deduplication.
+ // The duplicates might be from macro arguments that get expanded multiple
+ // times.
+ llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
+ const MissingIncludeDiagInfo &RHS) {
+ if (LHS.Symbol == RHS.Symbol) {
+ // We can get away just by comparing the offsets as all the ranges are in
+ // main file.
+ return LHS.SymRefRange.beginOffset() < RHS.SymRefRange.beginOffset();
+ }
+ // If symbols are different we don't care about the ordering.
+ return true;
+ });
+ MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
std::vector<const Inclusion *> UnusedIncludes =
getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
return {std::move(UnusedIncludes), std::move(MissingIncludes)};
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,36 @@
ParsedAST AST = TU.build();
auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
- // FIXME: Deduplicate references resulting from expansion of the same macro in
- // multiple places.
- EXPECT_THAT(Findings, testing::SizeIs(2));
+ EXPECT_THAT(Findings, testing::SizeIs(1));
auto RefRange = Findings.front().SymRefRange;
auto &SM = AST.getSourceManager();
EXPECT_EQ(RefRange.file(), SM.getMainFileID());
// FIXME: Point at the spelling location, rather than the include.
EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
- EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+ Annotations MainFile(R"cpp(
+ #include "all.h"
+ FOO([[Foo]]);
+ )cpp");
+
+ TestTU TU;
+ TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+ TU.AdditionalFiles["all.h"] = guard(R"cpp(
+ #include "foo.h"
+ #define FOO(X) X y; X z
+ )cpp");
+
+ TU.Code = MainFile.code();
+ ParsedAST AST = TU.build();
+
+ auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+ EXPECT_THAT(Findings, testing::SizeIs(1));
+ auto RefRange = Findings.front().SymRefRange;
+ auto &SM = AST.getSourceManager();
+ EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+ EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
}
TEST(IncludeCleaner, NoCrash) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -40,6 +40,7 @@
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/GenericUniformityImpl.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallString.h"
@@ -415,6 +416,20 @@
Ref.Target, TouchingTokens.back().range(SM), Providers};
MissingIncludes.push_back(std::move(DiagInfo));
});
+ // Put possibly equal diagnostics together for deduplication.
+ // The duplicates might be from macro arguments that get expanded multiple
+ // times.
+ llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
+ const MissingIncludeDiagInfo &RHS) {
+ if (LHS.Symbol == RHS.Symbol) {
+ // We can get away just by comparing the offsets as all the ranges are in
+ // main file.
+ return LHS.SymRefRange.beginOffset() < RHS.SymRefRange.beginOffset();
+ }
+ // If symbols are different we don't care about the ordering.
+ return true;
+ });
+ MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
std::vector<const Inclusion *> UnusedIncludes =
getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
return {std::move(UnusedIncludes), std::move(MissingIncludes)};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
