ilya-biryukov created this revision.
ilya-biryukov added a reviewer: hokein.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, javed.absar.
Will be used for updating the dynamic index on updates to the open files.
Currently we collect only information coming from the preamble
AST. This has a bunch of limitations:
- Dynamic index misses important information from the body of the file, e.g.
locations of definitions.
- XRefs cannot be collected at all, since we can only obtain full information
for the current file (preamble is parsed with skipped function bodies,
therefore not reliable).
This patch only adds the new callback, actually updating the index
will be added in a follow-up patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50847
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
unittests/clangd/TUSchedulerTests.cpp
Index: unittests/clangd/TUSchedulerTests.cpp
===================================================================
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -17,10 +17,11 @@
namespace clang {
namespace clangd {
+namespace {
using ::testing::_;
-using ::testing::Each;
using ::testing::AnyOf;
+using ::testing::Each;
using ::testing::Pair;
using ::testing::Pointee;
using ::testing::UnorderedElementsAre;
@@ -30,6 +31,18 @@
handleAllErrors(std::move(Err), [](const llvm::ErrorInfoBase &) {});
}
+class NoopParsingCallbacks : public ParsingCallbacks {
+public:
+ static NoopParsingCallbacks& instance() {
+ static NoopParsingCallbacks* Instance = new NoopParsingCallbacks;
+ return *Instance;
+ }
+
+ void onPreambleAST(PathRef Path, ASTContext &Ctx,
+ std::shared_ptr<clang::Preprocessor> PP) override {}
+ void onMainAST(PathRef Path, ParsedAST &AST) override {}
+};
+
class TUSchedulerTests : public ::testing::Test {
protected:
ParseInputs getInputs(PathRef File, std::string Contents) {
@@ -45,7 +58,7 @@
TEST_F(TUSchedulerTests, MissingFiles) {
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- /*PreambleParsedCallback=*/nullptr,
+ NoopParsingCallbacks::instance(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
@@ -102,7 +115,7 @@
TUScheduler S(
getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- /*PreambleParsedCallback=*/nullptr,
+ NoopParsingCallbacks::instance(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
auto Path = testPath("foo.cpp");
@@ -129,11 +142,10 @@
TEST_F(TUSchedulerTests, Debounce) {
std::atomic<int> CallbackCount(0);
{
- TUScheduler S(getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true,
- /*PreambleParsedCallback=*/nullptr,
- /*UpdateDebounce=*/std::chrono::seconds(1),
- ASTRetentionPolicy());
+ TUScheduler S(
+ getDefaultAsyncThreadsCount(),
+ /*StorePreamblesInMemory=*/true, NoopParsingCallbacks::instance(),
+ /*UpdateDebounce=*/std::chrono::seconds(1), ASTRetentionPolicy());
// FIXME: we could probably use timeouts lower than 1 second here.
auto Path = testPath("foo.cpp");
S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
@@ -163,7 +175,7 @@
{
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- /*PreambleParsedCallback=*/nullptr,
+ NoopParsingCallbacks::instance(),
/*UpdateDebounce=*/std::chrono::milliseconds(50),
ASTRetentionPolicy());
@@ -261,7 +273,7 @@
Policy.MaxRetainedASTs = 2;
TUScheduler S(
/*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
- PreambleParsedCallback(),
+ NoopParsingCallbacks::instance(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
llvm::StringLiteral SourceContents = R"cpp(
@@ -313,7 +325,7 @@
// the same time. All reads should get the same non-null preamble.
TUScheduler S(
/*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
- PreambleParsedCallback(),
+ NoopParsingCallbacks::instance(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
auto Foo = testPath("foo.cpp");
@@ -346,7 +358,7 @@
TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
TUScheduler S(
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+ /*StorePreambleInMemory=*/true, NoopParsingCallbacks::instance(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
@@ -399,7 +411,7 @@
TEST_F(TUSchedulerTests, NoChangeDiags) {
TUScheduler S(
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, PreambleParsedCallback(),
+ /*StorePreambleInMemory=*/true, NoopParsingCallbacks::instance(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
@@ -430,5 +442,6 @@
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
}
+} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -51,6 +51,21 @@
unsigned MaxRetainedASTs = 3;
};
+class ParsingCallbacks {
+public:
+ virtual ~ParsingCallbacks() = default;
+
+ /// Called on the AST that was built for emitting the preamble. The built AST
+ /// contains only AST nodes from the #include directives at the start of the
+ /// file. AST node in the current file should be observed on onMainAST call.
+ virtual void onPreambleAST(PathRef Path, ASTContext &Ctx,
+ std::shared_ptr<clang::Preprocessor> PP) = 0;
+ /// Called on the AST built for the file itself. Note that preamble AST nodes
+ /// are not deserialized and should be processed in the onPreambleAST call
+ /// instead.
+ virtual void onMainAST(PathRef Path, ParsedAST &AST) = 0;
+};
+
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -61,7 +76,7 @@
class TUScheduler {
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
- PreambleParsedCallback PreambleCallback,
+ ParsingCallbacks& ASTCallbacks,
std::chrono::steady_clock::duration UpdateDebounce,
ASTRetentionPolicy RetentionPolicy);
~TUScheduler();
@@ -132,7 +147,7 @@
private:
const bool StorePreamblesInMemory;
const std::shared_ptr<PCHContainerOperations> PCHOps;
- const PreambleParsedCallback PreambleCallback;
+ ParsingCallbacks& Callbacks;
Semaphore Barrier;
llvm::StringMap<std::unique_ptr<FileData>> Files;
std::unique_ptr<ASTCache> IdleASTs;
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -158,8 +158,7 @@
Semaphore &Barrier, bool RunSync,
steady_clock::duration UpdateDebounce,
std::shared_ptr<PCHContainerOperations> PCHs,
- bool StorePreamblesInMemory,
- PreambleParsedCallback PreambleCallback);
+ bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
public:
/// Create a new ASTWorker and return a handle to it.
@@ -173,7 +172,7 @@
steady_clock::duration UpdateDebounce,
std::shared_ptr<PCHContainerOperations> PCHs,
bool StorePreamblesInMemory,
- PreambleParsedCallback PreambleCallback);
+ ParsingCallbacks& Callbacks);
~ASTWorker();
void update(ParseInputs Inputs, WantDiagnostics,
@@ -228,8 +227,8 @@
const Path FileName;
/// Whether to keep the built preambles in memory or on disk.
const bool StorePreambleInMemory;
- /// Callback, passed to the preamble builder.
- const PreambleParsedCallback PreambleCallback;
+ /// Callback, invoked when preamble or main file AST is built.
+ ParsingCallbacks& Callbacks;
/// Helper class required to build the ASTs.
const std::shared_ptr<PCHContainerOperations> PCHs;
@@ -299,10 +298,10 @@
steady_clock::duration UpdateDebounce,
std::shared_ptr<PCHContainerOperations> PCHs,
bool StorePreamblesInMemory,
- PreambleParsedCallback PreambleCallback) {
+ ParsingCallbacks& Callbacks) {
std::shared_ptr<ASTWorker> Worker(new ASTWorker(
FileName, IdleASTs, Barrier, /*RunSync=*/!Tasks, UpdateDebounce,
- std::move(PCHs), StorePreamblesInMemory, std::move(PreambleCallback)));
+ std::move(PCHs), StorePreamblesInMemory, Callbacks));
if (Tasks)
Tasks->runAsync("worker:" + llvm::sys::path::filename(FileName),
[Worker]() { Worker->run(); });
@@ -315,10 +314,10 @@
steady_clock::duration UpdateDebounce,
std::shared_ptr<PCHContainerOperations> PCHs,
bool StorePreamblesInMemory,
- PreambleParsedCallback PreambleCallback)
+ ParsingCallbacks& Callbacks)
: IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory),
- PreambleCallback(std::move(PreambleCallback)), PCHs(std::move(PCHs)),
+ Callbacks(Callbacks), PCHs(std::move(PCHs)),
Barrier(Barrier), Done(false) {}
ASTWorker::~ASTWorker() {
@@ -365,7 +364,11 @@
getPossiblyStalePreamble();
std::shared_ptr<const PreambleData> NewPreamble =
buildPreamble(FileName, *Invocation, OldPreamble, OldCommand, Inputs,
- PCHs, StorePreambleInMemory, PreambleCallback);
+ PCHs, StorePreambleInMemory,
+ [this](PathRef Path, ASTContext &Ctx,
+ std::shared_ptr<clang::Preprocessor> PP) {
+ Callbacks.onPreambleAST(FileName, Ctx, std::move(PP));
+ });
bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble);
{
@@ -416,6 +419,7 @@
// Note *AST can be still be null if buildAST fails.
if (*AST) {
OnUpdated((*AST)->getDiagnostics());
+ Callbacks.onMainAST(FileName, **AST);
DiagsWereReported = true;
}
// Stash the AST in the cache for further use.
@@ -628,12 +632,12 @@
TUScheduler::TUScheduler(unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
- PreambleParsedCallback PreambleCallback,
+ ParsingCallbacks &Callbacks,
std::chrono::steady_clock::duration UpdateDebounce,
ASTRetentionPolicy RetentionPolicy)
: StorePreamblesInMemory(StorePreamblesInMemory),
- PCHOps(std::make_shared<PCHContainerOperations>()),
- PreambleCallback(std::move(PreambleCallback)), Barrier(AsyncThreadsCount),
+ PCHOps(std::make_shared<PCHContainerOperations>()), Callbacks(Callbacks),
+ Barrier(AsyncThreadsCount),
IdleASTs(llvm::make_unique<ASTCache>(RetentionPolicy.MaxRetainedASTs)),
UpdateDebounce(UpdateDebounce) {
if (0 < AsyncThreadsCount) {
@@ -671,8 +675,7 @@
// Create a new worker to process the AST-related tasks.
ASTWorkerHandle Worker = ASTWorker::create(
File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr,
- Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory,
- PreambleCallback);
+ Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, Callbacks);
FD = std::unique_ptr<FileData>(new FileData{
Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
} else {
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -223,6 +223,8 @@
SymbolIndex *Index;
// If present, an up-to-date of symbols in open files. Read via Index.
std::unique_ptr<FileIndex> FileIdx;
+ /// Callbacks responsible for updating FileIdx.
+ std::unique_ptr<ParsingCallbacks> FileIdxUpdater;
// If present, a merged view of FileIdx and an external index. Read via Index.
std::unique_ptr<SymbolIndex> MergedIndex;
// If set, this represents the workspace path.
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -66,6 +66,24 @@
Optional<Expected<tooling::AtomicChanges>> Result;
};
+class UpdateFileIndex : public ParsingCallbacks {
+public:
+ UpdateFileIndex(FileIndex *FileIdx) : FileIdx(FileIdx) {}
+
+ void onPreambleAST(PathRef Path, ASTContext &Ctx,
+ std::shared_ptr<clang::Preprocessor> PP) override {
+ if (FileIdx)
+ FileIdx->update(Path, &Ctx, std::move(PP));
+ }
+
+ void onMainAST(PathRef Path, ParsedAST &AST) override {
+ // FIXME: merge results from the main file into the index too.
+ }
+
+private:
+ FileIndex *FileIdx;
+};
+
} // namespace
ClangdServer::Options ClangdServer::optsForTest() {
@@ -85,20 +103,16 @@
: getStandardResourceDir()),
FileIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex(Opts.URISchemes)
: nullptr),
+ FileIdxUpdater(llvm::make_unique<UpdateFileIndex>(FileIdx.get())),
PCHs(std::make_shared<PCHContainerOperations>()),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
// parsed file and rebuild the file index synchronously each time an AST
// is parsed.
// FIXME(ioeric): this can be slow and we may be able to index on less
// critical paths.
- WorkScheduler(
- Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
- FileIdx
- ? [this](PathRef Path, ASTContext &AST,
- std::shared_ptr<Preprocessor>
- PP) { FileIdx->update(Path, &AST, std::move(PP)); }
- : PreambleParsedCallback(),
- Opts.UpdateDebounce, Opts.RetentionPolicy) {
+ WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
+ *FileIdxUpdater, Opts.UpdateDebounce,
+ Opts.RetentionPolicy) {
if (FileIdx && Opts.StaticIndex) {
MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex);
Index = MergedIndex.get();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
