Author: Kadir Cetinkaya Date: 2021-06-02T22:50:24+02:00 New Revision: 6c2a4e28f4d1c0f525c53302c08808c1b4f8073b
URL: https://github.com/llvm/llvm-project/commit/6c2a4e28f4d1c0f525c53302c08808c1b4f8073b DIFF: https://github.com/llvm/llvm-project/commit/6c2a4e28f4d1c0f525c53302c08808c1b4f8073b.diff LOG: [clangd] TUScheduler uses last active file for file-less queries This enables requests like workspaceSymbols to be dispatched using the file user was most recently operating on. A replacement for D103179. Differential Revision: https://reviews.llvm.org/D103476 Added: Modified: clang-tools-extra/clangd/TUScheduler.cpp clang-tools-extra/clangd/TUScheduler.h clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 7498598f124fb..09c68a3a250ba 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -1506,6 +1506,10 @@ bool TUScheduler::update(PathRef File, ParseInputs Inputs, 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::runQuick(llvm::StringRef Name, llvm::StringRef Path, 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 @@ void TUScheduler::runWithAST( "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 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File, ErrorCode::InvalidParams)); return; } + LastActiveFile = File.str(); if (!PreambleTasks) { trace::Span Tracer(Name); diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index 4731443d3d51b..02b602173ae67 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/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 @@ class TUScheduler { // 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 diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp index d68cb3efa3d6d..e15725d5120ba 100644 --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -1264,6 +1264,69 @@ TEST_F(TUSchedulerTests, IncluderCache) { << "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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits