[clang-tools-extra] r327537 - [clangd] Don't expose vfs in TUScheduler::runWithPreamble.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 14 10:46:52 PDT 2018
Author: ibiryukov
Date: Wed Mar 14 10:46:52 2018
New Revision: 327537
URL: http://llvm.org/viewvc/llvm-project?rev=327537&view=rev
Log:
[clangd] Don't expose vfs in TUScheduler::runWithPreamble.
Summary:
It was previously an easy way to concurrently access a mutable vfs,
which is a recipe for disaster.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, jkorous-apple, cfe-commits, ioeric
Differential Revision: https://reviews.llvm.org/D44463
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=327537&r1=327536&r2=327537&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Mar 14 10:46:52 2018
@@ -159,12 +159,11 @@ void ClangdServer::codeComplete(PathRef
llvm::Expected<InputsAndPreamble> IP) {
assert(IP && "error when trying to read preamble for codeComplete");
auto PreambleData = IP->Preamble;
- auto &Command = IP->Inputs.CompileCommand;
// FIXME(ibiryukov): even if Preamble is non-null, we may want to check
// both the old and the new version in case only one of them matches.
CompletionList Result = clangd::codeComplete(
- File, Command, PreambleData ? &PreambleData->Preamble : nullptr,
+ File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr,
Contents, Pos, FS, PCHs, CodeCompleteOpts);
CB(std::move(Result));
};
@@ -191,8 +190,7 @@ void ClangdServer::signatureHelp(PathRef
return CB(IP.takeError());
auto PreambleData = IP->Preamble;
- auto &Command = IP->Inputs.CompileCommand;
- CB(clangd::signatureHelp(File, Command,
+ CB(clangd::signatureHelp(File, IP->Command,
PreambleData ? &PreambleData->Preamble : nullptr,
Contents, Pos, FS, PCHs));
};
Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=327537&r1=327536&r2=327537&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Wed Mar 14 10:46:52 2018
@@ -407,7 +407,8 @@ unsigned getDefaultAsyncThreadsCount() {
struct TUScheduler::FileData {
/// Latest inputs, passed to TUScheduler::update().
- ParseInputs Inputs;
+ std::string Contents;
+ tooling::CompileCommand Command;
ASTWorkerHandle Worker;
};
@@ -456,9 +457,11 @@ void TUScheduler::update(PathRef File, P
File, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
CppFile(File, StorePreamblesInMemory, PCHOps, ASTCallback),
UpdateDebounce);
- FD = std::unique_ptr<FileData>(new FileData{Inputs, std::move(Worker)});
+ FD = std::unique_ptr<FileData>(new FileData{
+ Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
} else {
- FD->Inputs = Inputs;
+ FD->Contents = Inputs.Contents;
+ FD->Command = Inputs.CompileCommand;
}
FD->Worker->update(std::move(Inputs), WantDiags, std::move(OnUpdated));
}
@@ -500,26 +503,28 @@ void TUScheduler::runWithPreamble(
SPAN_ATTACH(Tracer, "file", File);
std::shared_ptr<const PreambleData> Preamble =
It->second->Worker->getPossiblyStalePreamble();
- Action(InputsAndPreamble{It->second->Inputs, Preamble.get()});
+ Action(InputsAndPreamble{It->second->Contents, It->second->Command,
+ Preamble.get()});
return;
}
- ParseInputs InputsCopy = It->second->Inputs;
std::shared_ptr<const ASTWorker> Worker = It->second->Worker.lock();
- auto Task = [InputsCopy, Worker, this](std::string Name, std::string File,
- Context Ctx,
- decltype(Action) Action) mutable {
+ auto Task = [Worker, this](std::string Name, std::string File,
+ std::string Contents,
+ tooling::CompileCommand Command, Context Ctx,
+ decltype(Action) Action) mutable {
std::lock_guard<Semaphore> BarrierLock(Barrier);
WithContext Guard(std::move(Ctx));
trace::Span Tracer(Name);
SPAN_ATTACH(Tracer, "file", File);
std::shared_ptr<const PreambleData> Preamble =
Worker->getPossiblyStalePreamble();
- Action(InputsAndPreamble{InputsCopy, Preamble.get()});
+ Action(InputsAndPreamble{Contents, Command, Preamble.get()});
};
PreambleTasks->runAsync("task:" + llvm::sys::path::filename(File),
Bind(Task, std::string(Name), std::string(File),
+ It->second->Contents, It->second->Command,
Context::current().clone(), std::move(Action)));
}
Modified: clang-tools-extra/trunk/clangd/TUScheduler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=327537&r1=327536&r2=327537&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.h (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.h Wed Mar 14 10:46:52 2018
@@ -29,7 +29,8 @@ struct InputsAndAST {
};
struct InputsAndPreamble {
- const ParseInputs &Inputs;
+ llvm::StringRef Contents;
+ const tooling::CompileCommand &Command;
const PreambleData *Preamble;
};
@@ -78,11 +79,14 @@ public:
void runWithAST(llvm::StringRef Name, PathRef File,
Callback<InputsAndAST> Action);
- /// Schedule an async read of the Preamble. Preamble passed to \p Action may
- /// be built for any version of the file, callers must not rely on it being
- /// consistent with the current version of the file.
- /// If an error occurs during processing, it is forwarded to the \p Action
- /// callback.
+ /// Schedule an async read of the Preamble.
+ /// The preamble may be stale, generated from an older version of the file.
+ /// Reading from locations in the preamble may cause the files to be re-read.
+ /// This gives callers two options:
+ /// - validate that the preamble is still valid, and only use it in this case
+ /// - accept that preamble contents may be outdated, and try to avoid reading
+ /// source code from headers. If an error occurs during processing, it is
+ /// forwarded to the \p Action callback.
void runWithPreamble(llvm::StringRef Name, PathRef File,
Callback<InputsAndPreamble> Action);
Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=327537&r1=327536&r2=327537&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Wed Mar 14 10:46:52 2018
@@ -226,8 +226,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
ASSERT_TRUE((bool)Preamble);
- EXPECT_EQ(Preamble->Inputs.FS, Inputs.FS);
- EXPECT_EQ(Preamble->Inputs.Contents, Inputs.Contents);
+ EXPECT_EQ(Preamble->Contents, Inputs.Contents);
std::lock_guard<std::mutex> Lock(Mut);
++TotalPreambleReads;
More information about the cfe-commits
mailing list