[clang-tools-extra] c31367e - [clangd] Build ASTs only with fresh preambles or after building a new preamble
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 12:20:56 PDT 2020
Author: Kadir Cetinkaya
Date: 2020-04-06T21:20:17+02:00
New Revision: c31367e95ce1e1e3fe97b1aee54a746f8dafc779
URL: https://github.com/llvm/llvm-project/commit/c31367e95ce1e1e3fe97b1aee54a746f8dafc779
DIFF: https://github.com/llvm/llvm-project/commit/c31367e95ce1e1e3fe97b1aee54a746f8dafc779.diff
LOG: [clangd] Build ASTs only with fresh preambles or after building a new preamble
Summary:
This is another step for out-of-order preamble builds. To keep the
diagnostic behavior same, we only build ASTs either with "usable" preambles,
the ones that are fully applicable to a given ParseInput, or after building a
new preamble. Which is the same behaviour as what we do today. ASTs
built in the latter is called golden ASTs.
Reviewers: sammccall
Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D76725
Added:
Modified:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index d234ec24535e..97cd22a5d1fc 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -90,7 +90,6 @@ PreambleData::PreambleData(const ParseInputs &Inputs,
std::shared_ptr<const PreambleData>
buildPreamble(PathRef FileName, CompilerInvocation CI,
- std::shared_ptr<const PreambleData> OldPreamble,
const ParseInputs &Inputs, bool StoreInMemory,
PreambleParsedCallback PreambleCallback) {
// Note that we don't need to copy the input contents, preamble can live
@@ -100,23 +99,6 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
auto Bounds =
ComputePreambleBounds(*CI.getLangOpts(), ContentsBuffer.get(), 0);
- if (OldPreamble &&
- compileCommandsAreEqual(Inputs.CompileCommand,
- OldPreamble->CompileCommand) &&
- OldPreamble->Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
- Inputs.FS.get())) {
- vlog("Reusing preamble version {0} for version {1} of {2}",
- OldPreamble->Version, Inputs.Version, FileName);
- return OldPreamble;
- }
- if (OldPreamble)
- vlog("Rebuilding invalidated preamble for {0} version {1} "
- "(previous was version {2})",
- FileName, Inputs.Version, OldPreamble->Version);
- else
- vlog("Building first preamble for {0} version {1}", FileName,
- Inputs.Version);
-
trace::Span Tracer("BuildPreamble");
SPAN_ATTACH(Tracer, "File", FileName);
StoreDiags PreambleDiagnostics;
@@ -172,5 +154,17 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
}
}
+bool isPreambleCompatible(const PreambleData &Preamble,
+ const ParseInputs &Inputs, PathRef FileName,
+ const CompilerInvocation &CI) {
+ auto ContentsBuffer =
+ llvm::MemoryBuffer::getMemBuffer(Inputs.Contents, FileName);
+ auto Bounds =
+ ComputePreambleBounds(*CI.getLangOpts(), ContentsBuffer.get(), 0);
+ return compileCommandsAreEqual(Inputs.CompileCommand,
+ Preamble.CompileCommand) &&
+ Preamble.Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
+ Inputs.FS.get());
+}
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h
index c42437beb705..bd67610e0ad7 100644
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -27,7 +27,9 @@
#include "Diagnostics.h"
#include "FS.h"
#include "Headers.h"
+#include "Path.h"
#include "index/CanonicalIncludes.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -72,17 +74,20 @@ using PreambleParsedCallback =
const CanonicalIncludes &)>;
/// Build a preamble for the new inputs unless an old one can be reused.
-/// If \p OldPreamble can be reused, it is returned unchanged.
-/// If \p OldPreamble is null, always builds the preamble.
/// If \p PreambleCallback is set, it will be run on top of the AST while
-/// building the preamble. Note that if the old preamble was reused, no AST is
-/// built and, therefore, the callback will not be executed.
+/// building the preamble.
std::shared_ptr<const PreambleData>
buildPreamble(PathRef FileName, CompilerInvocation CI,
- std::shared_ptr<const PreambleData> OldPreamble,
const ParseInputs &Inputs, bool StoreInMemory,
PreambleParsedCallback PreambleCallback);
+/// Returns true if \p Preamble is reusable for \p Inputs. Note that it will
+/// return true when some missing headers are now available.
+/// FIXME: Should return more information about the delta between \p Preamble
+/// and \p Inputs, e.g. new headers.
+bool isPreambleCompatible(const PreambleData &Preamble,
+ const ParseInputs &Inputs, PathRef FileName,
+ const CompilerInvocation &CI);
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp
index 2ca0ae855d56..bf847c1c86b4 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -5,41 +5,46 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-// For each file, managed by TUScheduler, we create a single ASTWorker that
-// manages an AST for that file. All operations that modify or read the AST are
-// run on a separate dedicated thread asynchronously in FIFO order.
+// TUScheduler manages a worker per active file. This ASTWorker processes
+// updates (modifications to file contents) and reads (actions performed on
+// preamble/AST) to the file.
//
-// We start processing each update immediately after we receive it. If two or
-// more updates come subsequently without reads in-between, we attempt to drop
-// an older one to not waste time building the ASTs we don't need.
+// Each ASTWorker owns a dedicated thread to process updates and reads to the
+// relevant file. Any request gets queued in FIFO order to be processed by that
+// thread.
//
-// The processing thread of the ASTWorker is also responsible for building the
-// preamble. However, unlike AST, the same preamble can be read concurrently, so
-// we run each of async preamble reads on its own thread.
+// An update request replaces current praser inputs to ensure any subsequent
+// read sees the version of the file they were requested. It will also issue a
+// build for new inputs.
//
-// To limit the concurrent load that clangd produces we maintain a semaphore
-// that keeps more than a fixed number of threads from running concurrently.
+// ASTWorker processes the file in two parts, a preamble and a main-file
+// section. A preamble can be reused between multiple versions of the file until
+// invalidated by a modification to a header, compile commands or modification
+// to relevant part of the current file. Such a preamble is called compatible.
+// An update is considered dead if no read was issued for that version and
+// diagnostics weren't requested by client or could be generated for a later
+// version of the file. ASTWorker eliminates such requests as they are
+// redundant.
//
-// Rationale for cancelling updates.
-// LSP clients can send updates to clangd on each keystroke. Some files take
-// significant time to parse (e.g. a few seconds) and clangd can get starved by
-// the updates to those files. Therefore we try to process only the last update,
-// if possible.
-// Our current strategy to do that is the following:
-// - For each update we immediately schedule rebuild of the AST.
-// - Rebuild of the AST checks if it was cancelled before doing any actual work.
-// If it was, it does not do an actual rebuild, only reports llvm::None to the
-// callback
-// - When adding an update, we cancel the last update in the queue if it didn't
-// have any reads.
-// There is probably a optimal ways to do that. One approach we might take is
-// the following:
-// - For each update we remember the pending inputs, but delay rebuild of the
-// AST for some timeout.
-// - If subsequent updates come before rebuild was started, we replace the
-// pending inputs and reset the timer.
-// - If any reads of the AST are scheduled, we start building the AST
-// immediately.
+// In the presence of stale (non-compatible) preambles, ASTWorker won't publish
+// diagnostics for update requests. Read requests will be served with ASTs build
+// with stale preambles, unless the read is picky and requires a compatible
+// preamble. In such cases it will block until new preamble is built.
+//
+// ASTWorker owns a PreambleThread for building preambles. If the preamble gets
+// invalidated by an update request, a new build will be requested on
+// PreambleThread. Since PreambleThread only receives requests for newer
+// versions of the file, in case of multiple requests it will only build the
+// last one and skip requests in between. Unless client force requested
+// diagnostics(WantDiagnostics::Yes).
+//
+// When a new preamble is built, a "golden" AST is immediately built from that
+// version of the file. This ensures diagnostics get updated even if the queue
+// is full.
+//
+// Some read requests might just need preamble. Since preambles can be read
+// concurrently, ASTWorker runs these requests on their own thread. These
+// requests will receive latest build preamble, which might possibly be stale.
#include "TUScheduler.h"
#include "Cancellation.h"
@@ -56,6 +61,8 @@
#include "index/CanonicalIncludes.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
@@ -64,14 +71,20 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Threading.h"
#include <algorithm>
+#include <chrono>
+#include <functional>
#include <memory>
#include <mutex>
#include <queue>
#include <string>
#include <thread>
+#include <type_traits>
+#include <utility>
+#include <vector>
namespace clang {
namespace clangd {
@@ -190,37 +203,26 @@ class SynchronizedTUStatus {
ParsingCallbacks &Callbacks;
};
-/// Responsible for building and providing access to the preamble of a TU.
-/// Whenever the thread is idle and the preamble is outdated, it starts to build
-/// a fresh preamble from the latest inputs. If RunSync is true, preambles are
-/// built synchronously in update() instead.
+/// Responsible for building preambles. Whenever the thread is idle and the
+/// preamble is outdated, it starts to build a fresh preamble from the latest
+/// inputs. If RunSync is true, preambles are built synchronously in update()
+/// instead.
class PreambleThread {
public:
PreambleThread(llvm::StringRef FileName, ParsingCallbacks &Callbacks,
bool StorePreambleInMemory, bool RunSync,
- SynchronizedTUStatus &Status)
+ SynchronizedTUStatus &Status, ASTWorker &AW)
: FileName(FileName), Callbacks(Callbacks),
- StoreInMemory(StorePreambleInMemory), RunSync(RunSync), Status(Status) {
- }
-
- size_t getUsedBytes() const {
- auto Preamble = latest();
- return Preamble ? Preamble->Preamble.getSize() : 0;
- }
+ StoreInMemory(StorePreambleInMemory), RunSync(RunSync), Status(Status),
+ ASTPeer(AW) {}
/// It isn't guaranteed that each requested version will be built. If there
/// are multiple update requests while building a preamble, only the last one
/// will be built.
- void update(CompilerInvocation *CI, ParseInputs PI) {
- // If compiler invocation was broken, just fail out early.
- if (!CI) {
- // Make sure anyone waiting for the preamble gets notified it could not be
- // built.
- BuiltFirst.notify();
- return;
- }
- // Make possibly expensive copy while not holding the lock.
- Request Req = {std::make_unique<CompilerInvocation>(*CI), std::move(PI)};
+ void update(std::unique_ptr<CompilerInvocation> CI, ParseInputs PI,
+ std::vector<Diag> CIDiags, WantDiagnostics WantDiags) {
+ Request Req = {std::move(CI), std::move(PI), std::move(CIDiags), WantDiags,
+ Context::current().clone()};
if (RunSync) {
build(std::move(Req));
Status.update([](TUStatus &Status) {
@@ -230,7 +232,9 @@ class PreambleThread {
}
{
std::lock_guard<std::mutex> Lock(Mutex);
- assert(!Done && "Build request to PreambleWorker after stop");
+ // If shutdown is issued, don't bother building.
+ if (Done)
+ return;
NextReq = std::move(Req);
}
// Let the worker thread know there's a request, notify_one is safe as there
@@ -238,19 +242,7 @@ class PreambleThread {
ReqCV.notify_all();
}
- /// Blocks until at least a single request has been processed. Note that it
- /// will unblock even after an unsuccessful build.
- void waitForFirst() const { BuiltFirst.wait(); }
-
- /// Returns the latest built preamble, might be null if no preamble has been
- /// built or latest attempt resulted in a failure.
- std::shared_ptr<const PreambleData> latest() const {
- std::lock_guard<std::mutex> Lock(Mutex);
- return LatestBuild;
- }
-
void run() {
- dlog("Starting preamble worker for {0}", FileName);
while (true) {
{
std::unique_lock<std::mutex> Lock(Mutex);
@@ -262,6 +254,8 @@ class PreambleThread {
CurrentReq = std::move(*NextReq);
NextReq.reset();
}
+
+ WithContext Guard(std::move(CurrentReq->Ctx));
// Build the preamble and let the waiters know about it.
build(std::move(*CurrentReq));
bool IsEmpty = false;
@@ -280,17 +274,16 @@ class PreambleThread {
}
ReqCV.notify_all();
}
- // We are no longer going to build any preambles, let the waiters know that.
- BuiltFirst.notify();
- dlog("Preamble worker for {0} finished", FileName);
+ dlog("Preamble worker for {0} stopped", FileName);
}
/// Signals the run loop to exit.
void stop() {
- dlog("Stopping preamble worker for {0}", FileName);
+ dlog("Preamble worker for {0} received stop", FileName);
{
std::lock_guard<std::mutex> Lock(Mutex);
Done = true;
+ NextReq.reset();
}
// Let the worker thread know that it should stop.
ReqCV.notify_all();
@@ -307,6 +300,9 @@ class PreambleThread {
struct Request {
std::unique_ptr<CompilerInvocation> CI;
ParseInputs Inputs;
+ std::vector<Diag> CIDiags;
+ WantDiagnostics WantDiags;
+ Context Ctx;
};
bool isDone() {
@@ -314,36 +310,9 @@ class PreambleThread {
return Done;
}
- /// Builds a preamble for Req and caches it. Might re-use the latest built
- /// preamble if it is valid for Req. Also signals waiters about the build.
- /// FIXME: We shouldn't cache failed preambles, if we've got a successful
- /// build before.
- void build(Request Req) {
- assert(Req.CI && "Got preamble request with null compiler invocation");
- const ParseInputs &Inputs = Req.Inputs;
- std::shared_ptr<const PreambleData> OldPreamble =
- Inputs.ForceRebuild ? nullptr : latest();
-
- Status.update([&](TUStatus &Status) {
- Status.PreambleActivity = PreambleAction::Building;
- });
-
- auto Preamble = clang::clangd::buildPreamble(
- FileName, std::move(*Req.CI), OldPreamble, Inputs, StoreInMemory,
- [this, Version(Inputs.Version)](
- ASTContext &Ctx, std::shared_ptr<clang::Preprocessor> PP,
- const CanonicalIncludes &CanonIncludes) {
- Callbacks.onPreambleAST(FileName, Version, Ctx, std::move(PP),
- CanonIncludes);
- });
- {
- std::lock_guard<std::mutex> Lock(Mutex);
- // LatestBuild might be the last reference to old preamble, do not trigger
- // destructor while holding the lock.
- std::swap(LatestBuild, Preamble);
- }
- BuiltFirst.notify();
- }
+ /// Builds a preamble for \p Req, might reuse LatestBuild if possible.
+ /// Notifies ASTWorker after build finishes.
+ void build(Request Req);
mutable std::mutex Mutex;
bool Done = false; /* GUARDED_BY(Mutex) */
@@ -351,16 +320,17 @@ class PreambleThread {
llvm::Optional<Request> CurrentReq; /* GUARDED_BY(Mutex) */
// Signaled whenever a thread populates NextReq or worker thread builds a
// Preamble.
- mutable std::condition_variable ReqCV; /* GUARDED_BY(Mutex) */
- std::shared_ptr<const PreambleData> LatestBuild; /* GUARDED_BY(Mutex) */
+ mutable std::condition_variable ReqCV; /* GUARDED_BY(Mutex) */
+ // Accessed only by preamble thread.
+ std::shared_ptr<const PreambleData> LatestBuild;
- Notification BuiltFirst;
const Path FileName;
ParsingCallbacks &Callbacks;
const bool StoreInMemory;
const bool RunSync;
SynchronizedTUStatus &Status;
+ ASTWorker &ASTPeer;
};
class ASTWorkerHandle;
@@ -404,6 +374,14 @@ class ASTWorker {
std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
+ /// Used to inform ASTWorker about a new preamble build by PreambleThread.
+ /// Diagnostics are only published through this callback. This ensures they
+ /// are always for newer versions of the file, as the callback gets called in
+ /// the same order as update requests.
+ void updatePreamble(std::unique_ptr<CompilerInvocation> CI, ParseInputs PI,
+ std::shared_ptr<const PreambleData> Preamble,
+ std::vector<Diag> CIDiags, WantDiagnostics WantDiags);
+
/// Obtain a preamble reflecting all updates so far. Threadsafe.
/// It may be delivered immediately, or later on the worker thread.
void getCurrentPreamble(
@@ -421,6 +399,12 @@ class ASTWorker {
bool isASTCached() const;
private:
+ /// Publishes diagnostics for \p Inputs. It will build an AST or reuse the
+ /// cached one if applicable. Assumes LatestPreamble is compatible for \p
+ /// Inputs.
+ void generateDiagnostics(std::unique_ptr<CompilerInvocation> Invocation,
+ ParseInputs Inputs, std::vector<Diag> CIDiags);
+
// Must be called exactly once on processing thread. Will return after
// stop() is called on a separate thread and all pending requests are
// processed.
@@ -464,6 +448,9 @@ class ASTWorker {
const GlobalCompilationDatabase &CDB;
/// Callback invoked when preamble or main file AST is built.
ParsingCallbacks &Callbacks;
+ /// Latest build preamble for current TU.
+ std::shared_ptr<const PreambleData> LatestPreamble;
+ Notification BuiltFirstPreamble;
Semaphore &Barrier;
/// Whether the 'onMainAST' callback ran for the current FileInputs.
@@ -480,6 +467,7 @@ class ASTWorker {
/// Set to true to signal run() to finish processing.
bool Done; /* GUARDED_BY(Mutex) */
std::deque<Request> Requests; /* GUARDED_BY(Mutex) */
+ std::queue<Request> PreambleRequests; /* GUARDED_BY(Mutex) */
llvm::Optional<Request> CurrentRequest; /* GUARDED_BY(Mutex) */
mutable std::condition_variable RequestsCV;
/// Guards the callback that publishes results of AST-related computations
@@ -494,7 +482,7 @@ class ASTWorker {
bool CanPublishResults = true; /* GUARDED_BY(PublishMu) */
SynchronizedTUStatus Status;
- PreambleThread PW;
+ PreambleThread PreamblePeer;
};
/// A smart-pointer-like class that points to an active ASTWorker.
@@ -551,7 +539,7 @@ ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB,
Tasks->runAsync("ASTWorker:" + llvm::sys::path::filename(FileName),
[Worker]() { Worker->run(); });
Tasks->runAsync("PreambleWorker:" + llvm::sys::path::filename(FileName),
- [Worker]() { Worker->PW.run(); });
+ [Worker]() { Worker->PreamblePeer.run(); });
}
return ASTWorkerHandle(std::move(Worker));
@@ -564,9 +552,10 @@ ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
: IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
FileName(FileName), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier),
Done(false), Status(FileName, Callbacks),
- // FIXME: Run preambleworker async.
- PW(FileName, Callbacks, StorePreamblesInMemory, /*RunSync=*/true,
- Status) {
+ PreamblePeer(FileName, Callbacks, StorePreamblesInMemory,
+ // FIXME: Run PreamblePeer asynchronously once ast patching
+ // is available.
+ /*RunSync=*/true, Status, *this) {
auto Inputs = std::make_shared<ParseInputs>();
// Set a fallback command because compile command can be accessed before
// `Inputs` is initialized. Other fields are only used after initialization
@@ -589,14 +578,6 @@ ASTWorker::~ASTWorker() {
void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
std::string TaskName = llvm::formatv("Update ({0})", Inputs.Version);
auto Task = [=]() mutable {
- auto RunPublish = [&](llvm::function_ref<void()> Publish) {
- // Ensure we only publish results from the worker if the file was not
- // removed, making sure there are not race conditions.
- std::lock_guard<std::mutex> Lock(PublishMu);
- if (CanPublishResults)
- Publish();
- };
-
// Get the actual command as `Inputs` does not have a command.
// FIXME: some build systems like Bazel will take time to preparing
// environment to build the file, it would be nice if we could emit a
@@ -607,33 +588,31 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
else
// FIXME: consider using old command if it's not a fallback one.
Inputs.CompileCommand = CDB.getFallbackCommand(FileName);
- auto PrevInputs = getCurrentFileInputs();
- // Will be used to check if we can avoid rebuilding the AST.
+
bool InputsAreTheSame =
- std::tie(PrevInputs->CompileCommand, PrevInputs->Contents) ==
+ std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
std::tie(Inputs.CompileCommand, Inputs.Contents);
+ // Cached AST is invalidated.
+ if (!InputsAreTheSame) {
+ IdleASTs.take(this);
+ RanASTCallback = false;
+ }
- bool RanCallbackForPrevInputs = RanASTCallback;
+ // Update current inputs so that subsequent reads can see them.
{
std::lock_guard<std::mutex> Lock(Mutex);
FileInputs = std::make_shared<ParseInputs>(Inputs);
}
- RanASTCallback = false;
+
log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
Inputs.CompileCommand.Directory,
llvm::join(Inputs.CompileCommand.CommandLine, " "));
- // Rebuild the preamble and the AST.
+
StoreDiags CompilerInvocationDiagConsumer;
std::vector<std::string> CC1Args;
std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
Inputs, CompilerInvocationDiagConsumer, &CC1Args);
- // This is true for now, as we always block until new preamble is build.
- // Once we start to block preambles out-of-order we need to make sure
- // OldPreamble refers to the preamble that was used to build last AST.
- auto OldPreamble = PW.latest();
- PW.update(Invocation.get(), Inputs);
- auto NewPreamble = PW.latest();
// Log cc1 args even (especially!) if creating invocation failed.
if (!CC1Args.empty())
vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " "));
@@ -643,109 +622,27 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
elog("Could not build CompilerInvocation for file {0}", FileName);
// Remove the old AST if it's still in cache.
IdleASTs.take(this);
+ RanASTCallback = false;
// Report the diagnostics we collected when parsing the command line.
Callbacks.onFailedAST(FileName, Inputs.Version,
- std::move(CompilerInvocationDiags), RunPublish);
- return;
- }
-
- bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble);
- // Before doing the expensive AST reparse, we want to release our reference
- // to the old preamble, so it can be freed if there are no other references
- // to it.
- OldPreamble.reset();
- Status.update([&](TUStatus &Status) {
- Status.ASTActivity.K = ASTAction::Building;
- Status.ASTActivity.Name = std::move(TaskName);
- });
- if (!CanReuseAST) {
- IdleASTs.take(this); // Remove the old AST if it's still in cache.
- } else {
- // We don't need to rebuild the AST, check if we need to run the callback.
- if (RanCallbackForPrevInputs) {
- RanASTCallback = true;
- // Take a shortcut and don't report the diagnostics, since they should
- // not changed. All the clients should handle the lack of OnUpdated()
- // call anyway to handle empty result from buildAST.
- // FIXME(ibiryukov): the AST could actually change if non-preamble
- // includes changed, but we choose to ignore it.
- // FIXME(ibiryukov): should we refresh the cache in IdleASTs for the
- // current file at this point?
- log("Skipping rebuild of the AST for {0}, inputs are the same.",
- FileName);
-
- Status.update([](TUStatus &Status) {
- Status.Details.ReuseAST = true;
- Status.Details.BuildFailed = false;
- });
- return;
- }
- }
-
- // We only need to build the AST if diagnostics were requested.
- if (WantDiags == WantDiagnostics::No)
+ std::move(CompilerInvocationDiags),
+ [&](llvm::function_ref<void()> Publish) {
+ // Ensure we only publish results from the worker
+ // if the file was not removed, making sure there
+ // are not race conditions.
+ std::lock_guard<std::mutex> Lock(PublishMu);
+ if (CanPublishResults)
+ Publish();
+ });
+ // Make sure anyone waiting for the preamble gets notified it could not be
+ // built.
+ BuiltFirstPreamble.notify();
return;
-
- {
- std::lock_guard<std::mutex> Lock(PublishMu);
- // No need to rebuild the AST if we won't send the diagnostics. However,
- // note that we don't prevent preamble rebuilds.
- if (!CanPublishResults)
- return;
}
- // Get the AST for diagnostics.
- llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
- auto RebuildStartTime = DebouncePolicy::clock::now();
- if (!AST) {
- llvm::Optional<ParsedAST> NewAST =
- buildAST(FileName, std::move(Invocation), CompilerInvocationDiags,
- Inputs, NewPreamble);
- // buildAST fails.
- Status.update([&](TUStatus &Status) {
- Status.Details.ReuseAST = false;
- Status.Details.BuildFailed = !NewAST.hasValue();
- });
- AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
- } else {
- // We are reusing the AST.
- Status.update([](TUStatus &Status) {
- Status.Details.ReuseAST = true;
- Status.Details.BuildFailed = false;
- });
- }
-
- // We want to report the diagnostics even if this update was cancelled.
- // It seems more useful than making the clients wait indefinitely if they
- // spam us with updates.
- // Note *AST can still be null if buildAST fails.
- if (*AST) {
- {
- // Try to record the AST-build time, to inform future update debouncing.
- // This is best-effort only: if the lock is held, don't bother.
- auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
- std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock);
- if (Lock.owns_lock()) {
- // Do not let RebuildTimes grow beyond its small-size (i.e. capacity).
- if (RebuildTimes.size() == RebuildTimes.capacity())
- RebuildTimes.erase(RebuildTimes.begin());
- RebuildTimes.push_back(RebuildDuration);
- }
- }
- trace::Span Span("Running main AST callback");
-
- Callbacks.onMainAST(FileName, **AST, RunPublish);
- RanASTCallback = true;
- } else {
- // Failed to build the AST, at least report diagnostics from the command
- // line if there were any.
- // FIXME: we might have got more errors while trying to build the AST,
- // surface them too.
- Callbacks.onFailedAST(FileName, Inputs.Version, CompilerInvocationDiags,
- RunPublish);
- }
- // Stash the AST in the cache for further use.
- IdleASTs.put(this, std::move(*AST));
+ PreamblePeer.update(std::move(Invocation), std::move(Inputs),
+ std::move(CompilerInvocationDiags), WantDiags);
+ return;
};
startTask(TaskName, std::move(Task), WantDiags, TUScheduler::NoInvalidation);
}
@@ -766,6 +663,9 @@ void ASTWorker::runWithAST(
// Try rebuilding the AST.
vlog("ASTWorker rebuilding evicted AST to run {0}: {1} version {2}", Name,
FileName, CurrentInputs->Version);
+ // FIXME: We might need to build a patched ast once preamble thread starts
+ // running async. Currently getPossiblyStalePreamble below will always
+ // return a compatible preamble as ASTWorker::update blocks.
llvm::Optional<ParsedAST> NewAST =
Invocation ? buildAST(FileName, std::move(Invocation),
CompilerInvocationDiagConsumer.take(),
@@ -787,9 +687,184 @@ void ASTWorker::runWithAST(
startTask(Name, std::move(Task), /*UpdateType=*/None, Invalidation);
}
+void PreambleThread::build(Request Req) {
+ assert(Req.CI && "Got preamble request with null compiler invocation");
+ const ParseInputs &Inputs = Req.Inputs;
+
+ Status.update([&](TUStatus &Status) {
+ Status.PreambleActivity = PreambleAction::Building;
+ });
+ auto _ = llvm::make_scope_exit([this, &Req] {
+ ASTPeer.updatePreamble(std::move(Req.CI), std::move(Req.Inputs),
+ LatestBuild, std::move(Req.CIDiags),
+ std::move(Req.WantDiags));
+ });
+
+ if (!LatestBuild || Inputs.ForceRebuild) {
+ vlog("Building first preamble for {0} version {1}", FileName,
+ Inputs.Version);
+ } else if (isPreambleCompatible(*LatestBuild, Inputs, FileName, *Req.CI)) {
+ vlog("Reusing preamble version {0} for version {1} of {2}",
+ LatestBuild->Version, Inputs.Version, FileName);
+ return;
+ } else {
+ vlog("Rebuilding invalidated preamble for {0} version {1} (previous was "
+ "version {2})",
+ FileName, Inputs.Version, LatestBuild->Version);
+ }
+
+ LatestBuild = clang::clangd::buildPreamble(
+ FileName, *Req.CI, Inputs, StoreInMemory,
+ [this, Version(Inputs.Version)](ASTContext &Ctx,
+ std::shared_ptr<clang::Preprocessor> PP,
+ const CanonicalIncludes &CanonIncludes) {
+ Callbacks.onPreambleAST(FileName, Version, Ctx, std::move(PP),
+ CanonIncludes);
+ });
+}
+
+void ASTWorker::updatePreamble(std::unique_ptr<CompilerInvocation> CI,
+ ParseInputs PI,
+ std::shared_ptr<const PreambleData> Preamble,
+ std::vector<Diag> CIDiags,
+ WantDiagnostics WantDiags) {
+ std::string TaskName = llvm::formatv("Build AST for ({0})", PI.Version);
+ // Store preamble and build diagnostics with new preamble if requested.
+ auto Task = [this, Preamble = std::move(Preamble), CI = std::move(CI),
+ PI = std::move(PI), CIDiags = std::move(CIDiags),
+ WantDiags = std::move(WantDiags)]() mutable {
+ // Update the preamble inside ASTWorker queue to ensure atomicity. As a task
+ // running inside ASTWorker assumes internals won't change until it
+ // finishes.
+ if (Preamble != LatestPreamble) {
+ // Cached AST is no longer valid.
+ IdleASTs.take(this);
+ RanASTCallback = false;
+ std::lock_guard<std::mutex> Lock(Mutex);
+ // LatestPreamble might be the last reference to old preamble, do not
+ // trigger destructor while holding the lock.
+ std::swap(LatestPreamble, Preamble);
+ }
+ // Give up our ownership to old preamble before starting expensive AST
+ // build.
+ Preamble.reset();
+ BuiltFirstPreamble.notify();
+ // We only need to build the AST if diagnostics were requested.
+ if (WantDiags == WantDiagnostics::No)
+ return;
+ // Report diagnostics with the new preamble to ensure progress. Otherwise
+ // diagnostics might get stale indefinitely if user keeps invalidating the
+ // preamble.
+ generateDiagnostics(std::move(CI), std::move(PI), std::move(CIDiags));
+ };
+ if (RunSync) {
+ Task();
+ return;
+ }
+ {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ PreambleRequests.push({std::move(Task), std::move(TaskName),
+ steady_clock::now(), Context::current().clone(),
+ llvm::None, TUScheduler::NoInvalidation, nullptr});
+ }
+ RequestsCV.notify_all();
+}
+
+void ASTWorker::generateDiagnostics(
+ std::unique_ptr<CompilerInvocation> Invocation, ParseInputs Inputs,
+ std::vector<Diag> CIDiags) {
+ assert(Invocation);
+ // No need to rebuild the AST if we won't send the diagnostics.
+ {
+ std::lock_guard<std::mutex> Lock(PublishMu);
+ if (!CanPublishResults)
+ return;
+ }
+ // Used to check whether we can update AST cache.
+ bool InputsAreLatest =
+ std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
+ std::tie(Inputs.CompileCommand, Inputs.Contents);
+ // Take a shortcut and don't report the diagnostics, since they should be the
+ // same. All the clients should handle the lack of OnUpdated() call anyway to
+ // handle empty result from buildAST.
+ // FIXME: the AST could actually change if non-preamble includes changed,
+ // but we choose to ignore it.
+ if (InputsAreLatest && RanASTCallback)
+ return;
+
+ // Get the AST for diagnostics, either build it or use the cached one.
+ std::string TaskName = llvm::formatv("Build AST ({0})", Inputs.Version);
+ Status.update([&](TUStatus &Status) {
+ Status.ASTActivity.K = ASTAction::Building;
+ Status.ASTActivity.Name = std::move(TaskName);
+ });
+ // We might be able to reuse the last we've built for a read request.
+ // FIXME: It might be better to not reuse this AST. That way queued AST builds
+ // won't be required for diags.
+ llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
+ if (!AST) {
+ auto RebuildStartTime = DebouncePolicy::clock::now();
+ llvm::Optional<ParsedAST> NewAST = buildAST(
+ FileName, std::move(Invocation), CIDiags, Inputs, LatestPreamble);
+ auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
+ // Try to record the AST-build time, to inform future update debouncing.
+ // This is best-effort only: if the lock is held, don't bother.
+ std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock);
+ if (Lock.owns_lock()) {
+ // Do not let RebuildTimes grow beyond its small-size (i.e.
+ // capacity).
+ if (RebuildTimes.size() == RebuildTimes.capacity())
+ RebuildTimes.erase(RebuildTimes.begin());
+ RebuildTimes.push_back(RebuildDuration);
+ Lock.unlock();
+ }
+ Status.update([&](TUStatus &Status) {
+ Status.Details.ReuseAST = false;
+ Status.Details.BuildFailed = !NewAST.hasValue();
+ });
+ AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
+ } else {
+ assert(InputsAreLatest && !RanASTCallback &&
+ "forgot to invalidate cached ast?");
+ log("Skipping rebuild of the AST for {0}, inputs are the same.", FileName);
+ Status.update([](TUStatus &Status) {
+ Status.Details.ReuseAST = true;
+ Status.Details.BuildFailed = false;
+ });
+ }
+
+ // Publish diagnostics.
+ auto RunPublish = [&](llvm::function_ref<void()> Publish) {
+ // Ensure we only publish results from the worker if the file was not
+ // removed, making sure there are not race conditions.
+ std::lock_guard<std::mutex> Lock(PublishMu);
+ if (CanPublishResults)
+ Publish();
+ };
+ if (*AST) {
+ trace::Span Span("Running main AST callback");
+ Callbacks.onMainAST(FileName, **AST, RunPublish);
+ } else {
+ // Failed to build the AST, at least report diagnostics from the
+ // command line if there were any.
+ // FIXME: we might have got more errors while trying to build the
+ // AST, surface them too.
+ Callbacks.onFailedAST(FileName, Inputs.Version, CIDiags, RunPublish);
+ }
+
+ // AST might've been built for an older version of the source, as ASTWorker
+ // queue raced ahead while we were waiting on the preamble. In that case the
+ // queue can't reuse the AST.
+ if (InputsAreLatest) {
+ RanASTCallback = *AST != nullptr;
+ IdleASTs.put(this, std::move(*AST));
+ }
+}
+
std::shared_ptr<const PreambleData>
ASTWorker::getPossiblyStalePreamble() const {
- return PW.latest();
+ std::lock_guard<std::mutex> Lock(Mutex);
+ return LatestPreamble;
}
void ASTWorker::getCurrentPreamble(
@@ -822,7 +897,7 @@ void ASTWorker::getCurrentPreamble(
RequestsCV.notify_all();
}
-void ASTWorker::waitForFirstPreamble() const { PW.waitForFirst(); }
+void ASTWorker::waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
std::shared_ptr<const ParseInputs> ASTWorker::getCurrentFileInputs() const {
std::unique_lock<std::mutex> Lock(Mutex);
@@ -856,6 +931,9 @@ void ASTWorker::stop() {
assert(!Done && "stop() called twice");
Done = true;
}
+ // We are no longer going to build any preambles, let the waiters know that.
+ BuiltFirstPreamble.notify();
+ PreamblePeer.stop();
Status.stop();
RequestsCV.notify_all();
}
@@ -899,13 +977,14 @@ void ASTWorker::startTask(llvm::StringRef Name,
}
void ASTWorker::run() {
- auto _ = llvm::make_scope_exit([this] { PW.stop(); });
while (true) {
{
std::unique_lock<std::mutex> Lock(Mutex);
assert(!CurrentRequest && "A task is already running, multiple workers?");
for (auto Wait = scheduleLocked(); !Wait.expired();
Wait = scheduleLocked()) {
+ assert(PreambleRequests.empty() &&
+ "Preamble updates should be scheduled immediately");
if (Done) {
if (Requests.empty())
return;
@@ -934,8 +1013,15 @@ void ASTWorker::run() {
wait(Lock, RequestsCV, Wait);
}
- CurrentRequest = std::move(Requests.front());
- Requests.pop_front();
+ // Any request in ReceivedPreambles is at least as old as the
+ // Requests.front(), so prefer them first to preserve LSP order.
+ if (!PreambleRequests.empty()) {
+ CurrentRequest = std::move(PreambleRequests.front());
+ PreambleRequests.pop();
+ } else {
+ CurrentRequest = std::move(Requests.front());
+ Requests.pop_front();
+ }
} // unlock Mutex
// It is safe to perform reads to CurrentRequest without holding the lock as
@@ -962,7 +1048,7 @@ void ASTWorker::run() {
{
std::lock_guard<std::mutex> Lock(Mutex);
CurrentRequest.reset();
- IsEmpty = Requests.empty();
+ IsEmpty = Requests.empty() && PreambleRequests.empty();
}
if (IsEmpty) {
Status.update([&](TUStatus &Status) {
@@ -975,6 +1061,9 @@ void ASTWorker::run() {
}
Deadline ASTWorker::scheduleLocked() {
+ // Process new preambles immediately.
+ if (!PreambleRequests.empty())
+ return Deadline::zero();
if (Requests.empty())
return Deadline::infinity(); // Wait for new requests.
// Handle cancelled requests first so the rest of the scheduler doesn't.
@@ -1045,8 +1134,9 @@ bool ASTWorker::shouldSkipHeadLocked() const {
bool ASTWorker::blockUntilIdle(Deadline Timeout) const {
std::unique_lock<std::mutex> Lock(Mutex);
- return wait(Lock, RequestsCV, Timeout,
- [&] { return Requests.empty() && !CurrentRequest; });
+ return wait(Lock, RequestsCV, Timeout, [&] {
+ return PreambleRequests.empty() && Requests.empty() && !CurrentRequest;
+ });
}
// Render a TUAction to a user-facing string representation.
@@ -1221,6 +1311,9 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File,
// Future is populated if the task needs a specific preamble.
std::future<std::shared_ptr<const PreambleData>> ConsistentPreamble;
+ // FIXME: Currently this only holds because ASTWorker blocks after issuing a
+ // preamble build. Get rid of consistent reads or make them build on the
+ // calling thread instead.
if (Consistency == Consistent) {
std::promise<std::shared_ptr<const PreambleData>> Promise;
ConsistentPreamble = Promise.get_future();
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index b873f91d3a4b..794b21ee61a5 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -112,9 +112,8 @@ CodeCompleteResult completions(const TestTU &TU, Position Point,
ADD_FAILURE() << "Couldn't build CompilerInvocation";
return {};
}
- auto Preamble =
- buildPreamble(testPath(TU.Filename), *CI, /*OldPreamble=*/nullptr, Inputs,
- /*InMemory=*/true, /*Callback=*/nullptr);
+ auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs,
+ /*InMemory=*/true, /*Callback=*/nullptr);
return codeComplete(testPath(TU.Filename), Inputs.CompileCommand,
Preamble.get(), TU.Code, Point, Inputs.FS, Opts);
}
@@ -518,16 +517,16 @@ TEST(CompletionTest, Kinds) {
AllOf(Named("complete_static_member"),
Kind(CompletionItemKind::Property))));
- Results = completions(
+ Results = completions(
R"cpp(
enum Color {
Red
};
Color u = ^
)cpp");
- EXPECT_THAT(Results.Completions,
- Contains(
- AllOf(Named("Red"), Kind(CompletionItemKind::EnumMember))));
+ EXPECT_THAT(
+ Results.Completions,
+ Contains(AllOf(Named("Red"), Kind(CompletionItemKind::EnumMember))));
}
TEST(CompletionTest, NoDuplicates) {
@@ -1046,9 +1045,8 @@ SignatureHelp signatures(llvm::StringRef Text, Position Point,
ADD_FAILURE() << "Couldn't build CompilerInvocation";
return {};
}
- auto Preamble =
- buildPreamble(testPath(TU.Filename), *CI, /*OldPreamble=*/nullptr, Inputs,
- /*InMemory=*/true, /*Callback=*/nullptr);
+ auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs,
+ /*InMemory=*/true, /*Callback=*/nullptr);
if (!Preamble) {
ADD_FAILURE() << "Couldn't build Preamble";
return {};
@@ -1712,7 +1710,7 @@ TEST(CompletionTest, FixItForArrowToDot) {
CodeCompleteOptions Opts;
Opts.IncludeFixIts = true;
- const char* Code =
+ const char *Code =
R"cpp(
class Auxilary {
public:
@@ -1746,7 +1744,7 @@ TEST(CompletionTest, FixItForArrowToDot) {
TEST(CompletionTest, FixItForDotToArrow) {
CodeCompleteOptions Opts;
Opts.IncludeFixIts = true;
- const char* Code =
+ const char *Code =
R"cpp(
class Auxilary {
public:
@@ -1850,7 +1848,7 @@ TEST(CompletionTest, CompletionTokenRange) {
R"cpp(
#include "foo/abc/[[fo^o.h"]]
)cpp",
- };
+ };
for (const auto &Text : TestCodes) {
Annotations TestCode(Text);
TU.Code = TestCode.code().str();
@@ -2222,10 +2220,9 @@ TEST(CompletionTest, NoInsertIncludeIfOnePresent) {
Sym.IncludeHeaders.emplace_back("\"foo.h\"", 2);
Sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
- EXPECT_THAT(
- completions(TU, Test.point(), {Sym}).Completions,
- UnorderedElementsAre(
- AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude()))));
+ EXPECT_THAT(completions(TU, Test.point(), {Sym}).Completions,
+ UnorderedElementsAre(AllOf(Named("Func"), HasInclude("\"foo.h\""),
+ Not(InsertInclude()))));
}
TEST(CompletionTest, MergeMacrosFromIndexAndSema) {
diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
index b020c2f72105..5203fb67307c 100644
--- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -286,7 +286,7 @@ TEST(FileIndexTest, RebuildWithPreamble) {
FileIndex Index;
bool IndexUpdated = false;
- buildPreamble(FooCpp, *CI, /*OldPreamble=*/nullptr, PI,
+ buildPreamble(FooCpp, *CI, PI,
/*StoreInMemory=*/true,
[&](ASTContext &Ctx, std::shared_ptr<Preprocessor> PP,
const CanonicalIncludes &CanonIncludes) {
@@ -424,7 +424,7 @@ TEST(FileIndexTest, ReferencesInMainFileWithPreamble) {
}
TEST(FileIndexTest, MergeMainFileSymbols) {
- const char* CommonHeader = "void foo();";
+ const char *CommonHeader = "void foo();";
TestTU Header = TestTU::withCode(CommonHeader);
TestTU Cpp = TestTU::withCode("void foo() {}");
Cpp.Filename = "foo.cpp";
diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 760e1c083353..0be66884fd66 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -24,6 +24,7 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <algorithm>
+#include <atomic>
#include <bits/stdint-uintn.h>
#include <chrono>
#include <utility>
@@ -283,6 +284,7 @@ TEST_F(TUSchedulerTests, PreambleConsistency) {
S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
[&](Expected<InputsAndPreamble> Pre) {
ASSERT_TRUE(bool(Pre));
+ ASSERT_TRUE(Pre->Preamble);
EXPECT_EQ(Pre->Preamble->Version, "A");
EXPECT_THAT(includes(Pre->Preamble),
ElementsAre("<A>"));
@@ -292,11 +294,13 @@ TEST_F(TUSchedulerTests, PreambleConsistency) {
S.runWithPreamble("ConsistentRead", Path, TUScheduler::Consistent,
[&](Expected<InputsAndPreamble> Pre) {
ASSERT_TRUE(bool(Pre));
+ ASSERT_TRUE(Pre->Preamble);
EXPECT_EQ(Pre->Preamble->Version, "B");
EXPECT_THAT(includes(Pre->Preamble),
ElementsAre("<B>"));
++CallbackCount;
});
+ S.blockUntilIdle(timeoutSeconds(10));
}
EXPECT_EQ(2, CallbackCount);
}
@@ -731,11 +735,14 @@ TEST_F(TUSchedulerTests, ForceRebuild) {
)cpp";
ParseInputs Inputs = getInputs(Source, SourceContents);
+ std::atomic<size_t> DiagCount(0);
// Update the source contents, which should trigger an initial build with
// the header file missing.
updateWithDiags(
- S, Source, Inputs, WantDiagnostics::Yes, [](std::vector<Diag> Diags) {
+ S, Source, Inputs, WantDiagnostics::Yes,
+ [&DiagCount](std::vector<Diag> Diags) {
+ ++DiagCount;
EXPECT_THAT(Diags,
ElementsAre(Field(&Diag::Message, "'foo.h' file not found"),
Field(&Diag::Message,
@@ -751,18 +758,23 @@ TEST_F(TUSchedulerTests, ForceRebuild) {
// The addition of the missing header file shouldn't trigger a rebuild since
// we don't track missing files.
updateWithDiags(
- S, Source, Inputs, WantDiagnostics::Yes, [](std::vector<Diag> Diags) {
+ S, Source, Inputs, WantDiagnostics::Yes,
+ [&DiagCount](std::vector<Diag> Diags) {
+ ++DiagCount;
ADD_FAILURE() << "Did not expect diagnostics for missing header update";
});
// Forcing the reload should should cause a rebuild which no longer has any
// errors.
Inputs.ForceRebuild = true;
- updateWithDiags(
- S, Source, Inputs, WantDiagnostics::Yes,
- [](std::vector<Diag> Diags) { EXPECT_THAT(Diags, IsEmpty()); });
+ updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
+ [&DiagCount](std::vector<Diag> Diags) {
+ ++DiagCount;
+ EXPECT_THAT(Diags, IsEmpty());
+ });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+ EXPECT_EQ(DiagCount, 2U);
}
TEST_F(TUSchedulerTests, NoChangeDiags) {
TUScheduler S(CDB, optsForTest(), captureDiags());
@@ -853,15 +865,19 @@ TEST_F(TUSchedulerTests, TUStatus) {
TUState(PreambleAction::Idle, ASTAction::RunningAction),
// We build the preamble
TUState(PreambleAction::Building, ASTAction::RunningAction),
- // Preamble worker goes idle
+ // We built the preamble, and issued ast built on ASTWorker
+ // thread. Preambleworker goes idle afterwards.
+ TUState(PreambleAction::Idle, ASTAction::RunningAction),
+ // Start task for building the ast, as a result of building
+ // preamble, on astworker thread.
TUState(PreambleAction::Idle, ASTAction::RunningAction),
- // We start building the ast
+ // AST build starts.
TUState(PreambleAction::Idle, ASTAction::Building),
- // Built finished succesffully
+ // AST built finished successfully
TUState(PreambleAction::Idle, ASTAction::Building),
- // Rnning go to def
+ // Running go to def
TUState(PreambleAction::Idle, ASTAction::RunningAction),
- // both workers go idle
+ // ASTWorker goes idle.
TUState(PreambleAction::Idle, ASTAction::Idle)));
}
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 2adcfc338cc2..fe3e8f01cca8 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -35,7 +35,7 @@ ParseInputs TestTU::inputs() const {
Files[ImportThunk] = ThunkContents;
ParseInputs Inputs;
- auto& Argv = Inputs.CompileCommand.CommandLine;
+ auto &Argv = Inputs.CompileCommand.CommandLine;
Argv = {"clang"};
// FIXME: this shouldn't need to be conditional, but it breaks a
// GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
@@ -71,8 +71,7 @@ ParsedAST TestTU::build() const {
auto CI = buildCompilerInvocation(Inputs, Diags);
assert(CI && "Failed to build compilation invocation.");
auto Preamble =
- buildPreamble(testPath(Filename), *CI,
- /*OldPreamble=*/nullptr, Inputs,
+ buildPreamble(testPath(Filename), *CI, Inputs,
/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
auto AST = buildAST(testPath(Filename), std::move(CI), Diags.take(), Inputs,
Preamble);
@@ -89,7 +88,7 @@ ParsedAST TestTU::build() const {
if (llvm::StringRef(Code).contains(Marker) ||
llvm::StringRef(HeaderCode).contains(Marker))
return true;
- for (const auto& KV : this->AdditionalFiles)
+ for (const auto &KV : this->AdditionalFiles)
if (llvm::StringRef(KV.second).contains(Marker))
return true;
return false;
More information about the cfe-commits
mailing list