[clang-tools-extra] r327532 - [clangd] Remove forceReparse, add a flag to addDocument instead
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 14 10:08:41 PDT 2018
Author: ibiryukov
Date: Wed Mar 14 10:08:41 2018
New Revision: 327532
URL: http://llvm.org/viewvc/llvm-project?rev=327532&view=rev
Log:
[clangd] Remove forceReparse, add a flag to addDocument instead
Summary: To make the removal of DraftMgr from ClangdServer easier (D44408).
Reviewers: sammccall, simark
Reviewed By: sammccall, simark
Subscribers: simark, klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44462
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=327532&r1=327531&r2=327532&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Mar 14 10:08:41 2018
@@ -116,10 +116,19 @@ void ClangdServer::setRootPath(PathRef R
}
void ClangdServer::addDocument(PathRef File, StringRef Contents,
- WantDiagnostics WantDiags) {
+ WantDiagnostics WantDiags, bool SkipCache) {
+ if (SkipCache)
+ CompileArgs.invalidate(File);
+
DocVersion Version = DraftMgr.updateDraft(File, Contents);
- scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()},
- WantDiags, FSProvider.getFileSystem());
+ ParseInputs Inputs = {CompileArgs.getCompileCommand(File),
+ FSProvider.getFileSystem(), Contents.str()};
+
+ Path FileStr = File.str();
+ WorkScheduler.update(File, std::move(Inputs), WantDiags,
+ [this, FileStr, Version](std::vector<Diag> Diags) {
+ consumeDiagnostics(FileStr, Version, std::move(Diags));
+ });
}
void ClangdServer::removeDocument(PathRef File) {
@@ -128,19 +137,6 @@ void ClangdServer::removeDocument(PathRe
WorkScheduler.remove(File);
}
-void ClangdServer::forceReparse(PathRef File) {
- auto FileContents = DraftMgr.getDraft(File);
- assert(FileContents.Draft &&
- "forceReparse() was called for non-added document");
-
- // forceReparse promises to request new compilation flags from CDB, so we
- // remove any cahced flags.
- CompileArgs.invalidate(File);
-
- scheduleReparseAndDiags(File, std::move(FileContents), WantDiagnostics::Yes,
- FSProvider.getFileSystem());
-}
-
void ClangdServer::codeComplete(PathRef File, Position Pos,
const clangd::CodeCompleteOptions &Opts,
Callback<CompletionList> CB) {
@@ -335,8 +331,8 @@ ClangdServer::insertInclude(PathRef File
// Replacement with offset UINT_MAX and length 0 will be treated as include
// insertion.
tooling::Replacement R(File, /*Offset=*/UINT_MAX, 0, "#include " + ToInclude);
- auto Replaces = format::cleanupAroundReplacements(
- Code, tooling::Replacements(R), *Style);
+ auto Replaces =
+ format::cleanupAroundReplacements(Code, tooling::Replacements(R), *Style);
if (!Replaces)
return Replaces;
return formatReplacements(Code, *Replaces, *Style);
@@ -499,39 +495,28 @@ void ClangdServer::findHover(PathRef Fil
WorkScheduler.runWithAST("Hover", File, Bind(Action, std::move(CB)));
}
-void ClangdServer::scheduleReparseAndDiags(
- PathRef File, VersionedDraft Contents, WantDiagnostics WantDiags,
- IntrusiveRefCntPtr<vfs::FileSystem> FS) {
- tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
-
- DocVersion Version = Contents.Version;
- Path FileStr = File.str();
-
- auto Callback = [this, Version, FileStr](std::vector<Diag> Diags) {
- // We need to serialize access to resulting diagnostics to avoid calling
- // `onDiagnosticsReady` in the wrong order.
- std::lock_guard<std::mutex> DiagsLock(DiagnosticsMutex);
- DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr];
- // FIXME(ibiryukov): get rid of '<' comparison here. In the current
- // implementation diagnostics will not be reported after version counters'
- // overflow. This should not happen in practice, since DocVersion is a
- // 64-bit unsigned integer.
- if (Version < LastReportedDiagsVersion)
- return;
- LastReportedDiagsVersion = Version;
-
- DiagConsumer.onDiagnosticsReady(FileStr, std::move(Diags));
- };
+void ClangdServer::consumeDiagnostics(PathRef File, DocVersion Version,
+ std::vector<Diag> Diags) {
+ // We need to serialize access to resulting diagnostics to avoid calling
+ // `onDiagnosticsReady` in the wrong order.
+ std::lock_guard<std::mutex> DiagsLock(DiagnosticsMutex);
+ DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[File];
+
+ // FIXME(ibiryukov): get rid of '<' comparison here. In the current
+ // implementation diagnostics will not be reported after version counters'
+ // overflow. This should not happen in practice, since DocVersion is a
+ // 64-bit unsigned integer.
+ if (Version < LastReportedDiagsVersion)
+ return;
+ LastReportedDiagsVersion = Version;
- WorkScheduler.update(File,
- ParseInputs{std::move(Command), std::move(FS),
- std::move(*Contents.Draft)},
- WantDiags, std::move(Callback));
+ DiagConsumer.onDiagnosticsReady(File, std::move(Diags));
}
void ClangdServer::reparseOpenedFiles() {
for (const Path &FilePath : DraftMgr.getActiveFiles())
- forceReparse(FilePath);
+ addDocument(FilePath, *DraftMgr.getDraft(FilePath).Draft,
+ WantDiagnostics::Auto, /*SkipCache=*/true);
}
void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=327532&r1=327531&r2=327532&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Mar 14 10:08:41 2018
@@ -117,22 +117,20 @@ public:
/// \p File is already tracked. Also schedules parsing of the AST for it on a
/// separate thread. When the parsing is complete, DiagConsumer passed in
/// constructor will receive onDiagnosticsReady callback.
+ /// When \p SkipCache is true, compile commands will always be requested from
+ /// compilation database even if they were cached in previous invocations.
void addDocument(PathRef File, StringRef Contents,
- WantDiagnostics WD = WantDiagnostics::Auto);
+ WantDiagnostics WD = WantDiagnostics::Auto,
+ bool SkipCache = false);
/// Remove \p File from list of tracked files, schedule a request to free
/// resources associated with it.
void removeDocument(PathRef File);
- /// Force \p File to be reparsed using the latest contents.
- /// Will also check if CompileCommand, provided by GlobalCompilationDatabase
- /// for \p File has changed. If it has, will remove currently stored Preamble
- /// and AST and rebuild them from scratch.
- void forceReparse(PathRef File);
-
/// Calls forceReparse() on all currently opened files.
/// As a result, this method may be very expensive.
/// This method is normally called when the compilation database is changed.
+ /// FIXME: this method must be moved to ClangdLSPServer along with DraftMgr.
void reparseOpenedFiles();
/// Run code completion for \p File at \p Pos.
@@ -240,9 +238,8 @@ private:
formatCode(llvm::StringRef Code, PathRef File,
ArrayRef<tooling::Range> Ranges);
- void scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
- WantDiagnostics WD,
- IntrusiveRefCntPtr<vfs::FileSystem> FS);
+ void consumeDiagnostics(PathRef File, DocVersion Version,
+ std::vector<Diag> Diags);
CompileArgsCache CompileArgs;
DiagnosticsConsumer &DiagConsumer;
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=327532&r1=327531&r2=327532&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Wed Mar 14 10:08:41 2018
@@ -245,13 +245,13 @@ int b = a;
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
FS.Files[FooH] = "";
- Server.forceReparse(FooCpp);
+ Server.addDocument(FooCpp, SourceContents);
auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
FS.Files[FooH] = "int a;";
- Server.forceReparse(FooCpp);
+ Server.addDocument(FooCpp, SourceContents);
auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
@@ -368,15 +368,16 @@ struct bar { T x; };
// Now switch to C++ mode.
CDB.ExtraClangFlags = {"-xc++"};
- // Currently, addDocument never checks if CompileCommand has changed, so we
+ // By default addDocument does not check if CompileCommand has changed, so we
// expect to see the errors.
runAddDocument(Server, FooCpp, SourceContents1);
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
runAddDocument(Server, FooCpp, SourceContents2);
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
- // But forceReparse should reparse the file with proper flags.
- Server.forceReparse(FooCpp);
- ASSERT_TRUE(Server.blockUntilIdleForTest());
+ // Passing SkipCache=true will force addDocument to reparse the file with
+ // proper flags.
+ runAddDocument(Server, FooCpp, SourceContents2, WantDiagnostics::Auto,
+ /*SkipCache=*/true);
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
// Subsequent addDocument calls should finish without errors too.
runAddDocument(Server, FooCpp, SourceContents1);
@@ -408,12 +409,14 @@ int main() { return 0; }
// Parse without the define, no errors should be produced.
CDB.ExtraClangFlags = {};
- // Currently, addDocument never checks if CompileCommand has changed, so we
+ // By default addDocument does not check if CompileCommand has changed, so we
// expect to see the errors.
runAddDocument(Server, FooCpp, SourceContents);
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
- // But forceReparse should reparse the file with proper flags.
- Server.forceReparse(FooCpp);
+ // Passing SkipCache=true will force addDocument to reparse the file with
+ // proper flags.
+ runAddDocument(Server, FooCpp, SourceContents, WantDiagnostics::Auto,
+ /*SkipCache=*/true);
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
// Subsequent addDocument call should finish without errors too.
@@ -675,44 +678,31 @@ int d;
Stats.FileIsRemoved = true;
};
- auto UpdateStatsOnForceReparse = [&](unsigned FileIndex) {
- auto &Stats = ReqStats[FileIndex];
-
- if (Stats.LastContentsHadErrors)
- ++Stats.RequestsWithErrors;
- else
- ++Stats.RequestsWithoutErrors;
- };
-
- auto AddDocument = [&](unsigned FileIndex) {
+ auto AddDocument = [&](unsigned FileIndex, bool SkipCache) {
bool ShouldHaveErrors = ShouldHaveErrorsDist(RandGen);
Server.addDocument(FilePaths[FileIndex],
ShouldHaveErrors ? SourceContentsWithErrors
- : SourceContentsWithoutErrors);
+ : SourceContentsWithoutErrors,
+ WantDiagnostics::Auto, SkipCache);
UpdateStatsOnAddDocument(FileIndex, ShouldHaveErrors);
};
// Various requests that we would randomly run.
auto AddDocumentRequest = [&]() {
unsigned FileIndex = FileIndexDist(RandGen);
- AddDocument(FileIndex);
+ AddDocument(FileIndex, /*SkipCache=*/false);
};
auto ForceReparseRequest = [&]() {
unsigned FileIndex = FileIndexDist(RandGen);
- // Make sure we don't violate the ClangdServer's contract.
- if (ReqStats[FileIndex].FileIsRemoved)
- AddDocument(FileIndex);
-
- Server.forceReparse(FilePaths[FileIndex]);
- UpdateStatsOnForceReparse(FileIndex);
+ AddDocument(FileIndex, /*SkipCache=*/true);
};
auto RemoveDocumentRequest = [&]() {
unsigned FileIndex = FileIndexDist(RandGen);
// Make sure we don't violate the ClangdServer's contract.
if (ReqStats[FileIndex].FileIsRemoved)
- AddDocument(FileIndex);
+ AddDocument(FileIndex, /*SkipCache=*/false);
Server.removeDocument(FilePaths[FileIndex]);
UpdateStatsOnRemoveDocument(FileIndex);
@@ -722,7 +712,7 @@ int d;
unsigned FileIndex = FileIndexDist(RandGen);
// Make sure we don't violate the ClangdServer's contract.
if (ReqStats[FileIndex].FileIsRemoved)
- AddDocument(FileIndex);
+ AddDocument(FileIndex, /*SkipCache=*/false);
Position Pos;
Pos.line = LineDist(RandGen);
@@ -741,7 +731,7 @@ int d;
unsigned FileIndex = FileIndexDist(RandGen);
// Make sure we don't violate the ClangdServer's contract.
if (ReqStats[FileIndex].FileIsRemoved)
- AddDocument(FileIndex);
+ AddDocument(FileIndex, /*SkipCache=*/false);
Position Pos;
Pos.line = LineDist(RandGen);
Modified: clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp?rev=327532&r1=327531&r2=327532&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp Wed Mar 14 10:08:41 2018
@@ -11,8 +11,9 @@
namespace clang {
namespace clangd {
-void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents) {
- Server.addDocument(File, Contents);
+void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents,
+ WantDiagnostics WantDiags, bool SkipCache) {
+ Server.addDocument(File, Contents, WantDiags, SkipCache);
if (!Server.blockUntilIdleForTest())
llvm_unreachable("not idle after addDocument");
}
Modified: clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SyncAPI.h?rev=327532&r1=327531&r2=327532&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SyncAPI.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/SyncAPI.h Wed Mar 14 10:08:41 2018
@@ -19,7 +19,9 @@ namespace clang {
namespace clangd {
// Calls addDocument and then blockUntilIdleForTest.
-void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents);
+void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents,
+ WantDiagnostics WantDiags = WantDiagnostics::Auto,
+ bool SkipCache = false);
llvm::Expected<CompletionList>
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
More information about the cfe-commits
mailing list