kadircet updated this revision to Diff 349366.
kadircet marked an inline comment as done.
kadircet added a comment.
Keep using `Path` in `runWithSemaphore`, by substituting `LastActiveFile` when
empty.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103476/new/
https://reviews.llvm.org/D103476
Files:
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1264,6 +1264,69 @@
<< "association still valid";
}
+TEST_F(TUSchedulerTests, PreservesLastActiveFile) {
+ for (bool Sync : {false, true}) {
+ auto Opts = optsForTest();
+ if (Sync)
+ Opts.AsyncThreadsCount = 0;
+ TUScheduler S(CDB, Opts);
+
+ auto CheckNoFileActionsSeesLastActiveFile =
+ [&](llvm::StringRef LastActiveFile) {
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+ std::atomic<int> Counter(0);
+ // We only check for run and runQuick as runWithAST and
+ // runWithPreamble is always bound to a file.
+ S.run("run-UsesLastActiveFile", /*Path=*/"", [&] {
+ ++Counter;
+ EXPECT_EQ(LastActiveFile, boundPath());
+ });
+ S.runQuick("runQuick-UsesLastActiveFile", /*Path=*/"", [&] {
+ ++Counter;
+ EXPECT_EQ(LastActiveFile, boundPath());
+ });
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+ EXPECT_EQ(2, Counter.load());
+ };
+
+ // Check that we see no file initially
+ CheckNoFileActionsSeesLastActiveFile("");
+
+ // Now check that every action scheduled with a particular file changes the
+ // LastActiveFile.
+ auto Path = testPath("run.cc");
+ S.run(Path, Path, [] {});
+ CheckNoFileActionsSeesLastActiveFile(Path);
+
+ Path = testPath("runQuick.cc");
+ S.runQuick(Path, Path, [] {});
+ CheckNoFileActionsSeesLastActiveFile(Path);
+
+ Path = testPath("runWithAST.cc");
+ S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+ S.runWithAST(Path, Path, [](llvm::Expected<InputsAndAST> Inp) {
+ EXPECT_TRUE(bool(Inp));
+ });
+ CheckNoFileActionsSeesLastActiveFile(Path);
+
+ Path = testPath("runWithPreamble.cc");
+ S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+ S.runWithPreamble(
+ Path, Path, TUScheduler::Stale,
+ [](llvm::Expected<InputsAndPreamble> Inp) { EXPECT_TRUE(bool(Inp)); });
+ CheckNoFileActionsSeesLastActiveFile(Path);
+
+ Path = testPath("update.cc");
+ S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+ CheckNoFileActionsSeesLastActiveFile(Path);
+
+ // An update with the same contents should not change LastActiveFile.
+ auto LastActive = Path;
+ Path = testPath("runWithAST.cc");
+ S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+ CheckNoFileActionsSeesLastActiveFile(LastActive);
+ }
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -23,6 +23,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include <chrono>
+#include <string>
namespace clang {
namespace clangd {
@@ -341,6 +342,9 @@
// asynchronously.
llvm::Optional<AsyncTaskRunner> PreambleTasks;
llvm::Optional<AsyncTaskRunner> WorkerThreads;
+ // Used to create contexts for operations that are not bound to a particular
+ // file (e.g. index queries).
+ std::string LastActiveFile;
};
} // namespace clangd
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1506,6 +1506,10 @@
FD->Contents = Inputs.Contents;
}
FD->Worker->update(std::move(Inputs), WantDiags, ContentChanged);
+ // There might be synthetic update requests, don't change the LastActiveFile
+ // in such cases.
+ if (ContentChanged)
+ LastActiveFile = File.str();
return NewFile;
}
@@ -1535,6 +1539,10 @@
void TUScheduler::runWithSemaphore(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function<void()> Action,
Semaphore &Sem) {
+ if (Path.empty())
+ Path = LastActiveFile;
+ else
+ LastActiveFile = Path.str();
if (!PreambleTasks) {
WithContext WithProvidedContext(Opts.ContextProvider(Path));
return Action();
@@ -1559,6 +1567,7 @@
"trying to get AST for non-added document", ErrorCode::InvalidParams));
return;
}
+ LastActiveFile = File.str();
It->second->Worker->runWithAST(Name, std::move(Action), Invalidation);
}
@@ -1573,6 +1582,7 @@
ErrorCode::InvalidParams));
return;
}
+ LastActiveFile = File.str();
if (!PreambleTasks) {
trace::Span Tracer(Name);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
