kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113262
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
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
@@ -234,8 +234,8 @@
TU.Code = MainFile.str();
ParsedAST AST = TU.build();
std::vector<std::string> UnusedIncludes;
- for (const auto &Include : computeUnusedIncludes(AST))
- UnusedIncludes.push_back(Include->Written);
+ for (const auto &I : computeUnusedIncludes(AST))
+ UnusedIncludes.push_back(I.Include->Written);
EXPECT_THAT(UnusedIncludes,
UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\""));
}
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -66,7 +66,16 @@
getUnused(ParsedAST &AST,
const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles);
-std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
+llvm::StringSet<> reachableUsedHeaders(
+ IncludeStructure &IncludeGraph, IncludeStructure::HeaderID Source,
+ const llvm::DenseSet<IncludeStructure::HeaderID> &UsedHeaders);
+
+struct UnusedInclude {
+ const Inclusion *Include;
+ llvm::StringSet<> ValidReplacements;
+};
+
+std::vector<UnusedInclude> computeUnusedIncludes(ParsedAST &AST);
std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
llvm::StringRef Code);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -8,6 +8,7 @@
#include "IncludeCleaner.h"
#include "Config.h"
+#include "Headers.h"
#include "ParsedAST.h"
#include "Protocol.h"
#include "SourceCode.h"
@@ -18,8 +19,10 @@
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include <queue>
namespace clang {
namespace clangd {
@@ -242,28 +245,6 @@
return std::move(Result.Files);
}
-std::vector<const Inclusion *>
-getUnused(ParsedAST &AST,
- const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles) {
- trace::Span Tracer("IncludeCleaner::getUnused");
- std::vector<const Inclusion *> Unused;
- for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
- if (!MFI.HeaderID)
- continue;
- auto IncludeID = static_cast<IncludeStructure::HeaderID>(*MFI.HeaderID);
- bool Used = ReferencedFiles.contains(IncludeID);
- if (!Used && !mayConsiderUnused(MFI, AST)) {
- dlog("{0} was not used, but is not eligible to be diagnosed as unused",
- MFI.Written);
- continue;
- }
- if (!Used)
- Unused.push_back(&MFI);
- dlog("{0} is {1}", MFI.Written, Used ? "USED" : "UNUSED");
- }
- return Unused;
-}
-
#ifndef NDEBUG
// Is FID a <built-in>, <scratch space> etc?
static bool isSpecialBuffer(FileID FID, const SourceManager &SM) {
@@ -292,7 +273,49 @@
return TranslatedHeaderIDs;
}
-std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST) {
+std::vector<const Inclusion *>
+getUnused(ParsedAST &AST,
+ const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles) {
+ trace::Span Tracer("IncludeCleaner::getUnused");
+ std::vector<const Inclusion *> Unused;
+ for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
+ if (!MFI.HeaderID)
+ continue;
+ auto IncludeID = static_cast<IncludeStructure::HeaderID>(*MFI.HeaderID);
+ bool Used = ReferencedFiles.contains(IncludeID);
+ if (!Used && !mayConsiderUnused(MFI, AST)) {
+ dlog("{0} was not used, but is not eligible to be diagnosed as unused",
+ MFI.Written);
+ continue;
+ }
+ if (!Used)
+ Unused.push_back(&MFI);
+ dlog("{0} is {1}", MFI.Written, Used ? "USED" : "UNUSED");
+ }
+ return Unused;
+}
+
+llvm::StringSet<> reachableUsedHeaders(
+ const IncludeStructure &IncludeGraph, IncludeStructure::HeaderID Source,
+ const llvm::DenseSet<IncludeStructure::HeaderID> &UsedHeaders) {
+ llvm::StringSet<> Result;
+ llvm::DenseSet<IncludeStructure::HeaderID> Visited;
+ std::queue<IncludeStructure::HeaderID> ToVisit;
+ ToVisit.push(Source);
+ while (!ToVisit.empty()) {
+ auto NextID = ToVisit.front();
+ ToVisit.pop();
+ Visited.insert(NextID);
+ if (UsedHeaders.contains(NextID))
+ Result.insert(IncludeGraph.getRealPath(NextID));
+ for (const auto &Header : IncludeGraph.IncludeChildren.lookup(NextID))
+ if (!Visited.contains(Header))
+ ToVisit.push(Header);
+ }
+ return Result;
+}
+
+std::vector<UnusedInclude> computeUnusedIncludes(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
auto Refs = findReferencedLocations(AST);
@@ -304,7 +327,15 @@
auto ReferencedFileIDs = findReferencedFiles(Refs, SM);
auto ReferencedHeaders =
translateToHeaderIDs(ReferencedFileIDs, AST.getIncludeStructure(), SM);
- return getUnused(AST, ReferencedHeaders);
+ std::vector<UnusedInclude> UnusedIncludes;
+ for (const auto *Header : getUnused(AST, ReferencedHeaders)) {
+ UnusedIncludes.push_back(UnusedInclude{
+ Header, reachableUsedHeaders(
+ AST.getIncludeStructure(),
+ static_cast<IncludeStructure::HeaderID>(*Header->HeaderID),
+ ReferencedHeaders)});
+ }
+ return UnusedIncludes;
}
std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
@@ -321,19 +352,19 @@
.getFileEntryForID(AST.getSourceManager().getMainFileID())
->getName()
.str();
- for (const auto *Inc : computeUnusedIncludes(AST)) {
+ for (const auto &I : computeUnusedIncludes(AST)) {
Diag D;
- D.Message =
- llvm::formatv("included header {0} is not used",
- llvm::sys::path::filename(
- Inc->Written.substr(1, Inc->Written.size() - 2),
- llvm::sys::path::Style::posix));
+ D.Message = llvm::formatv(
+ "included header {0} is not used",
+ llvm::sys::path::filename(
+ I.Include->Written.substr(1, I.Include->Written.size() - 2),
+ llvm::sys::path::Style::posix));
D.Name = "unused-includes";
D.Source = Diag::DiagSource::Clangd;
D.File = FileName;
D.Severity = DiagnosticsEngine::Warning;
D.Tags.push_back(Unnecessary);
- D.Range = getDiagnosticRange(Code, Inc->HashOffset);
+ D.Range = getDiagnosticRange(Code, I.Include->HashOffset);
// FIXME(kirillbobyrev): Removing inclusion might break the code if the
// used headers are only reachable transitively through this one. Suggest
// including them directly instead.
@@ -342,8 +373,14 @@
D.Fixes.emplace_back();
D.Fixes.back().Message = "remove #include directive";
D.Fixes.back().Edits.emplace_back();
- D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
- D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
+ D.Fixes.back().Edits.back().range.start.line = I.Include->HashLine;
+ D.Fixes.back().Edits.back().range.end.line = I.Include->HashLine + 1;
+ std::string DirectIncludes;
+ for (const auto &Replacement : I.ValidReplacements) {
+ DirectIncludes +=
+ llvm::formatv("#include \"{0}\"\n", Replacement.getKey());
+ }
+ D.Fixes.back().Edits.back().newText = DirectIncludes;
D.InsideMainFile = true;
Result.push_back(std::move(D));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
