hokein updated this revision to Diff 490287.
hokein added a comment.
update the tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140875/new/
https://reviews.llvm.org/D140875
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
clang-tools-extra/include-cleaner/lib/Record.cpp
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -150,7 +150,7 @@
RecordPragma(const CompilerInstance &CI, PragmaIncludes *Out)
: SM(CI.getSourceManager()),
HeaderInfo(CI.getPreprocessor().getHeaderSearchInfo()), Out(Out),
- UniqueStrings(Arena) {}
+ UniqueStrings(Out->Arena) {}
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -176,7 +176,6 @@
std::unique(It.getSecond().begin(), It.getSecond().end()),
It.getSecond().end());
}
- Out->Arena = std::move(Arena);
}
void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
@@ -307,7 +306,6 @@
const SourceManager &SM;
HeaderSearch &HeaderInfo;
PragmaIncludes *Out;
- llvm::BumpPtrAllocator Arena;
/// Intern table for strings. Contents are on the arena.
llvm::StringSaver UniqueStrings;
@@ -336,6 +334,27 @@
std::vector<KeepPragma> KeepStack;
};
+PragmaIncludes::PragmaIncludes(const PragmaIncludes & In) {
+ *this = In;
+}
+
+PragmaIncludes &PragmaIncludes::operator=(const PragmaIncludes &In) {
+ ShouldKeep = In.ShouldKeep;
+ NonSelfContainedFiles = In.NonSelfContainedFiles;
+ Arena.Reset();
+ llvm::UniqueStringSaver Strings(Arena);
+ IWYUPublic.clear();
+ for (const auto& It : In.IWYUPublic)
+ IWYUPublic.try_emplace(It.first, Strings.save(It.second));
+ IWYUExportBy.clear();
+ for (const auto& UIDAndExporters : In.IWYUExportBy) {
+ const auto& [UID, Exporters] = UIDAndExporters;
+ for (StringRef ExporterHeader : Exporters)
+ IWYUExportBy.try_emplace(UID).first->second.push_back(ExporterHeader);
+ }
+ return *this;
+ }
+
void PragmaIncludes::record(const CompilerInstance &CI) {
auto Record = std::make_unique<RecordPragma>(CI, this);
CI.getPreprocessor().addCommentHandler(Record.get());
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -48,6 +48,13 @@
/// defines the symbol.
class PragmaIncludes {
public:
+ PragmaIncludes() = default;
+ PragmaIncludes(PragmaIncludes &&) = default;
+ PragmaIncludes &operator=(PragmaIncludes &&) = default;
+
+ PragmaIncludes(const PragmaIncludes &);
+ PragmaIncludes &operator=(const PragmaIncludes &);
+
/// Installs an analysing PPCallback and CommentHandler and populates results
/// to the structure.
void record(const CompilerInstance &CI);
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -343,6 +343,8 @@
auto Unused = computeUnusedIncludes(AST);
EXPECT_THAT(Unused, ElementsAre(Pointee(writtenInclusion("<queue>"))));
+ EXPECT_THAT(computeUnusedIncludesNew(AST),
+ ElementsAre(Pointee(writtenInclusion("<queue>"))));
}
TEST(IncludeCleaner, GetUnusedHeaders) {
@@ -379,6 +381,10 @@
UnusedIncludes.push_back(Include->Written);
EXPECT_THAT(UnusedIncludes,
UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\""));
+ EXPECT_THAT(
+ computeUnusedIncludesNew(AST),
+ UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
+ Pointee(writtenInclusion("\"dir/unused.h\""))));
}
TEST(IncludeCleaner, VirtualBuffers) {
@@ -533,6 +539,9 @@
// IWYU pragma: private, include "public.h"
void foo() {}
)cpp");
+ Config Cfg;
+ Cfg.Diagnostics.UnusedIncludes = Config::Experiment;
+ WithContextValue Ctx(Config::Key, std::move(Cfg));
ParsedAST AST = TU.build();
auto ReferencedFiles = findReferencedFiles(
@@ -547,6 +556,7 @@
ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
+ EXPECT_THAT(computeUnusedIncludesNew(AST), IsEmpty());
}
TEST(IncludeCleaner, RecursiveInclusion) {
@@ -575,6 +585,7 @@
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
+ EXPECT_THAT(computeUnusedIncludesNew(AST), IsEmpty());
}
TEST(IncludeCleaner, IWYUPragmaExport) {
@@ -599,6 +610,7 @@
// FIXME: This is not correct: foo.h is unused but is not diagnosed as such
// because we ignore headers with IWYU export pragmas for now.
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
+ EXPECT_THAT(computeUnusedIncludesNew(AST), IsEmpty());
}
TEST(IncludeCleaner, NoDiagsForObjC) {
Index: clang-tools-extra/clangd/Preamble.h
===================================================================
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -27,6 +27,7 @@
#include "Diagnostics.h"
#include "FS.h"
#include "Headers.h"
+#include "clang-include-cleaner/Record.h"
#include "index/CanonicalIncludes.h"
#include "support/Path.h"
#include "clang/Frontend/CompilerInvocation.h"
@@ -57,6 +58,8 @@
// Processes like code completions and go-to-definitions will need #include
// information, and their compile action skips preamble range.
IncludeStructure Includes;
+
+ include_cleaner::PragmaIncludes Pragmas;
// Macros defined in the preamble section of the main file.
// Users care about headers vs main-file, not preamble vs non-preamble.
// These should be treated as main-file entities e.g. for code completion.
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -11,6 +11,7 @@
#include "Config.h"
#include "Headers.h"
#include "SourceCode.h"
+#include "clang-include-cleaner/Record.h"
#include "support/Logger.h"
#include "support/ThreadsafeFS.h"
#include "support/Trace.h"
@@ -77,6 +78,9 @@
std::vector<PragmaMark> takeMarks() { return std::move(Marks); }
+ include_cleaner::PragmaIncludes takePragmaIncludes() {
+ return std::move(Pragmas);
+ }
CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); }
bool isMainFileIncludeGuarded() const { return IsMainFileIncludeGuarded; }
@@ -118,6 +122,9 @@
LangOpts = &CI.getLangOpts();
SourceMgr = &CI.getSourceManager();
Includes.collect(CI);
+ if (Config::current().Diagnostics.UnusedIncludes ==
+ Config::UnusedIncludesPolicy::Experiment)
+ Pragmas.record(CI);
if (BeforeExecuteCallback)
BeforeExecuteCallback(CI);
}
@@ -187,6 +194,7 @@
PreambleParsedCallback ParsedCallback;
IncludeStructure Includes;
CanonicalIncludes CanonIncludes;
+ include_cleaner::PragmaIncludes Pragmas;
MainFileMacros Macros;
std::vector<PragmaMark> Marks;
bool IsMainFileIncludeGuarded = false;
@@ -560,6 +568,7 @@
Result->CompileCommand = Inputs.CompileCommand;
Result->Diags = std::move(Diags);
Result->Includes = CapturedInfo.takeIncludes();
+ Result->Pragmas = CapturedInfo.takePragmaIncludes();
Result->Macros = CapturedInfo.takeMacros();
Result->Marks = CapturedInfo.takeMarks();
Result->CanonIncludes = CapturedInfo.takeCanonicalIncludes();
Index: clang-tools-extra/clangd/ParsedAST.h
===================================================================
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -25,6 +25,7 @@
#include "Diagnostics.h"
#include "Headers.h"
#include "Preamble.h"
+#include "clang-include-cleaner/Record.h"
#include "index/CanonicalIncludes.h"
#include "support/Path.h"
#include "clang/Frontend/FrontendAction.h"
@@ -107,6 +108,10 @@
/// (!) does not have tokens from the preamble.
const syntax::TokenBuffer &getTokens() const { return Tokens; }
+ const include_cleaner::PragmaIncludes &getPragmaIncludes() const {
+ return Pragmas;
+ }
+
/// Returns the version of the ParseInputs this AST was built from.
llvm::StringRef version() const { return Version; }
@@ -129,7 +134,8 @@
MainFileMacros Macros, std::vector<PragmaMark> Marks,
std::vector<Decl *> LocalTopLevelDecls,
std::optional<std::vector<Diag>> Diags, IncludeStructure Includes,
- CanonicalIncludes CanonIncludes);
+ CanonicalIncludes CanonIncludes,
+ include_cleaner::PragmaIncludes Pragmas);
Path TUPath;
std::string Version;
@@ -161,6 +167,7 @@
std::vector<Decl *> LocalTopLevelDecls;
IncludeStructure Includes;
CanonicalIncludes CanonIncludes;
+ include_cleaner::PragmaIncludes Pragmas;
std::unique_ptr<HeuristicResolver> Resolver;
};
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -23,6 +23,7 @@
#include "Preamble.h"
#include "SourceCode.h"
#include "TidyProvider.h"
+#include "clang-include-cleaner/Record.h"
#include "index/CanonicalIncludes.h"
#include "index/Index.h"
#include "index/Symbol.h"
@@ -589,9 +590,11 @@
}
IncludeStructure Includes;
+ include_cleaner::PragmaIncludes Pragmas;
// If we are using a preamble, copy existing includes.
if (Preamble) {
Includes = Preamble->Includes;
+ Pragmas = Preamble->Pragmas;
Includes.MainFileIncludes = Patch->preambleIncludes();
// Replay the preamble includes so that clang-tidy checks can see them.
ReplayPreamble::attach(Patch->preambleIncludes(), *Clang,
@@ -601,6 +604,9 @@
// Otherwise we would collect the replayed includes again...
// (We can't *just* use the replayed includes, they don't have Resolved path).
Includes.collect(*Clang);
+ if (Config::current().Diagnostics.UnusedIncludes ==
+ Config::UnusedIncludesPolicy::Experiment)
+ Pragmas.record(*Clang);
// Copy over the macros in the preamble region of the main file, and combine
// with non-preamble macros below.
MainFileMacros Macros;
@@ -686,7 +692,7 @@
std::move(Clang), std::move(Action), std::move(Tokens),
std::move(Macros), std::move(Marks), std::move(ParsedDecls),
std::move(Diags), std::move(Includes),
- std::move(CanonIncludes));
+ std::move(CanonIncludes), std::move(Pragmas));
if (Result.Diags) {
auto UnusedHeadersDiags =
issueUnusedIncludesDiagnostics(Result, Inputs.Contents);
@@ -789,13 +795,15 @@
std::vector<PragmaMark> Marks,
std::vector<Decl *> LocalTopLevelDecls,
std::optional<std::vector<Diag>> Diags,
- IncludeStructure Includes, CanonicalIncludes CanonIncludes)
+ IncludeStructure Includes, CanonicalIncludes CanonIncludes,
+ include_cleaner::PragmaIncludes Pragmas)
: TUPath(TUPath), Version(Version), Preamble(std::move(Preamble)),
Clang(std::move(Clang)), Action(std::move(Action)),
Tokens(std::move(Tokens)), Macros(std::move(Macros)),
Marks(std::move(Marks)), Diags(std::move(Diags)),
LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
- Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
+ Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)),
+ Pragmas(std::move(Pragmas)) {
Resolver = std::make_unique<HeuristicResolver>(getASTContext());
assert(this->Clang);
assert(this->Action);
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -97,6 +97,7 @@
const llvm::StringSet<> &ReferencedPublicHeaders);
std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
+std::vector<const Inclusion *> computeUnusedIncludesNew(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
@@ -12,6 +12,8 @@
#include "ParsedAST.h"
#include "Protocol.h"
#include "SourceCode.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
#include "index/CanonicalIncludes.h"
#include "support/Logger.h"
#include "support/Trace.h"
@@ -458,6 +460,9 @@
return TranslatedHeaderIDs;
}
+// This is the original clangd-own implementation for computing unused
+// #includes. Eventually it will be deprecated and replaced by the
+// include-cleaner-library-based implementation.
std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
@@ -469,11 +474,63 @@
translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM);
return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas);
}
+// An include-cleaner-based implementation of computeUnusedIncludes.
+std::vector<const Inclusion *> computeUnusedIncludesNew(ParsedAST &AST) {
+ const auto &SM = AST.getSourceManager();
+ const auto &Includes = AST.getIncludeStructure();
+ // FIXME: this map should probably be in IncludeStructure.
+ llvm::StringMap<llvm::SmallVector<IncludeStructure::HeaderID>> BySpelling;
+ for (const auto &Inc : Includes.MainFileIncludes) {
+ if (Inc.HeaderID)
+ BySpelling.try_emplace(Inc.Written)
+ .first->second.push_back(
+ static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID));
+ }
+ // FIXME: !!this is a hacky way to collect macro references.
+ std::vector<include_cleaner::SymbolReference> Macros;
+ auto& PP = AST.getPreprocessor();
+ for (const syntax::Token &Tok :
+ AST.getTokens().spelledTokens(SM.getMainFileID())) {
+ auto Macro = locateMacroAt(Tok, PP);
+ if (!Macro)
+ continue;
+ if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
+ Macros.push_back(
+ {Tok.location(),
+ include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
+ DefLoc},
+ include_cleaner::RefType::Explicit});
+ }
+ llvm::DenseSet<IncludeStructure::HeaderID> Used;
+ include_cleaner::walkUsed(
+ AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
+ &AST.getPragmaIncludes(), SM,
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> Providers) {
+ for (const auto &H : Providers) {
+ switch (H.kind()) {
+ case include_cleaner::Header::Physical:
+ if (auto HeaderID = Includes.getID(H.physical()))
+ Used.insert(*HeaderID);
+ break;
+ case include_cleaner::Header::Standard:
+ for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard()))
+ Used.insert(HeaderID);
+ break;
+ case include_cleaner::Header::Verbatim:
+ for (auto HeaderID : BySpelling.lookup(H.verbatim()))
+ Used.insert(HeaderID);
+ break;
+ }
+ }
+ });
+ return getUnused(AST, Used, /*ReferencedPublicHeaders*/{});
+}
std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
llvm::StringRef Code) {
const Config &Cfg = Config::current();
- if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
+ if (Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::None ||
Cfg.Diagnostics.SuppressAll ||
Cfg.Diagnostics.Suppress.contains("unused-includes"))
return {};
@@ -487,7 +544,11 @@
.getFileEntryRefForID(AST.getSourceManager().getMainFileID())
->getName()
.str();
- for (const auto *Inc : computeUnusedIncludes(AST)) {
+ const auto &UnusedIncludes =
+ Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::Experiment
+ ? computeUnusedIncludesNew(AST)
+ : computeUnusedIncludes(AST);
+ for (const auto *Inc : UnusedIncludes) {
Diag D;
D.Message =
llvm::formatv("included header {0} is not used directly",
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -431,11 +431,13 @@
});
if (F.UnusedIncludes)
- if (auto Val = compileEnum<Config::UnusedIncludesPolicy>(
- "UnusedIncludes", **F.UnusedIncludes)
- .map("Strict", Config::UnusedIncludesPolicy::Strict)
- .map("None", Config::UnusedIncludesPolicy::None)
- .value())
+ if (auto Val =
+ compileEnum<Config::UnusedIncludesPolicy>("UnusedIncludes",
+ **F.UnusedIncludes)
+ .map("Strict", Config::UnusedIncludesPolicy::Strict)
+ .map("Experiment", Config::UnusedIncludesPolicy::Experiment)
+ .map("None", Config::UnusedIncludesPolicy::None)
+ .value())
Out.Apply.push_back([Val](const Params &, Config &C) {
C.Diagnostics.UnusedIncludes = *Val;
});
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -88,7 +88,13 @@
bool StandardLibrary = true;
} Index;
- enum UnusedIncludesPolicy { Strict, None };
+ enum UnusedIncludesPolicy {
+ /// Diagnose unused includes.
+ Strict,
+ None,
+ /// The same as Strict, but using the include-cleaner library.
+ Experiment,
+ };
/// Controls warnings and errors when parsing code.
struct {
bool SuppressAll = false;
Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -59,6 +59,7 @@
endif()
include_directories(BEFORE "${CMAKE_CURRENT_BINARY_DIR}/../clang-tidy")
+include_directories(BEFORE "${CMAKE_CURRENT_SOURCE_DIR}/../include-cleaner/include")
add_clang_library(clangDaemon
AST.cpp
@@ -162,6 +163,7 @@
clangDriver
clangFormat
clangFrontend
+ clangIncludeCleaner
clangIndex
clangLex
clangSema
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
