[clang-tools-extra] cf7160c - [clangd] Config: also propagate in sync (testing) mode

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-07-15T12:30:08+02:00
New Revision: cf7160c0b0c1250596cc9b2ba0e41423ac465a8f

URL: https://github.com/llvm/llvm-project/commit/cf7160c0b0c1250596cc9b2ba0e41423ac465a8f
DIFF: https://github.com/llvm/llvm-project/commit/cf7160c0b0c1250596cc9b2ba0e41423ac465a8f.diff

LOG: [clangd] Config: also propagate in sync (testing) mode

Summary:
I hit this while trying to add a config-over-LSP lit test, which I think
is an appropriate way to test this feature.

That needs a few more changes though...

Reviewers: kadircet

Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D83802

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 5454b1c92c8a..ed367005177b 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -965,6 +965,7 @@ void ASTWorker::startTask(llvm::StringRef Name,
   if (RunSync) {
     assert(!Done && "running a task after stop()");
     trace::Span Tracer(Name + ":" + llvm::sys::path::filename(FileName));
+    WithContext WithProvidedContext(ContextProvider(FileName));
     Task();
     return;
   }
@@ -1062,9 +1063,7 @@ void ASTWorker::run() {
         Status.ASTActivity.K = ASTAction::RunningAction;
         Status.ASTActivity.Name = CurrentRequest->Name;
       });
-      llvm::Optional<WithContext> WithProvidedContext;
-      if (ContextProvider)
-        WithProvidedContext.emplace(ContextProvider(FileName));
+      WithContext WithProvidedContext(ContextProvider(FileName));
       CurrentRequest->Action();
     }
 
@@ -1238,6 +1237,12 @@ TUScheduler::TUScheduler(const GlobalCompilationDatabase &CDB,
       Barrier(Opts.AsyncThreadsCount),
       IdleASTs(
           std::make_unique<ASTCache>(Opts.RetentionPolicy.MaxRetainedASTs)) {
+  // Avoid null checks everywhere.
+  if (!Opts.ContextProvider) {
+    this->Opts.ContextProvider = [](llvm::StringRef) {
+      return Context::current().clone();
+    };
+  }
   if (0 < Opts.AsyncThreadsCount) {
     PreambleTasks.emplace();
     WorkerThreads.emplace();
@@ -1300,16 +1305,16 @@ llvm::StringMap<std::string> TUScheduler::getAllFileContents() const {
 
 void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path,
                       llvm::unique_function<void()> Action) {
-  if (!PreambleTasks)
+  if (!PreambleTasks) {
+    WithContext WithProvidedContext(Opts.ContextProvider(Path));
     return Action();
+  }
   PreambleTasks->runAsync(Name, [this, Ctx = Context::current().clone(),
                                  Path(Path.str()),
                                  Action = std::move(Action)]() mutable {
     std::lock_guard<Semaphore> BarrierLock(Barrier);
     WithContext WC(std::move(Ctx));
-    llvm::Optional<WithContext> WithProvidedContext;
-    if (Opts.ContextProvider)
-      WithProvidedContext.emplace(Opts.ContextProvider(Path));
+    WithContext WithProvidedContext(Opts.ContextProvider(Path));
     Action();
   });
 }
@@ -1344,6 +1349,7 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File,
     SPAN_ATTACH(Tracer, "file", File);
     std::shared_ptr<const PreambleData> Preamble =
         It->second->Worker->getPossiblyStalePreamble();
+    WithContext WithProvidedContext(Opts.ContextProvider(File));
     Action(InputsAndPreamble{It->second->Contents,
                              It->second->Worker->getCurrentCompileCommand(),
                              Preamble.get()});
@@ -1370,9 +1376,7 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, PathRef File,
         WithContext Guard(std::move(Ctx));
         trace::Span Tracer(Name);
         SPAN_ATTACH(Tracer, "file", File);
-        llvm::Optional<WithContext> WithProvidedContext;
-        if (Opts.ContextProvider)
-          WithProvidedContext.emplace(Opts.ContextProvider(File));
+        WithContext WithProvidedContext(Opts.ContextProvider(File));
         Action(InputsAndPreamble{Contents, Command, Preamble.get()});
       };
 

diff  --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h
index 05c06da13380..5d545b366ec3 100644
--- a/clang-tools-extra/clangd/TUScheduler.h
+++ b/clang-tools-extra/clangd/TUScheduler.h
@@ -313,7 +313,7 @@ class TUScheduler {
 
 private:
   const GlobalCompilationDatabase &CDB;
-  const Options Opts;
+  Options Opts;
   std::unique_ptr<ParsingCallbacks> Callbacks; // not nullptr
   Semaphore Barrier;
   llvm::StringMap<std::unique_ptr<FileData>> Files;

diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index f40377fd5d85..61ac4f7a27a4 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -864,25 +864,28 @@ TEST_F(TUSchedulerTests, NoChangeDiags) {
 }
 
 TEST_F(TUSchedulerTests, Run) {
-  auto Opts = optsForTest();
-  Opts.ContextProvider = bindPath;
-  TUScheduler S(CDB, Opts);
-  std::atomic<int> Counter(0);
-  S.run("add 1", /*Path=*/"", [&] { ++Counter; });
-  S.run("add 2", /*Path=*/"", [&] { Counter += 2; });
-  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
-  EXPECT_EQ(Counter.load(), 3);
-
-  Notification TaskRun;
-  Key<int> TestKey;
-  WithContextValue CtxWithKey(TestKey, 10);
-  const char *Path = "somepath";
-  S.run("props context", Path, [&] {
-    EXPECT_EQ(Context::current().getExisting(TestKey), 10);
-    EXPECT_EQ(Path, boundPath());
-    TaskRun.notify();
-  });
-  TaskRun.wait();
+  for (bool Sync : {false, true}) {
+    auto Opts = optsForTest();
+    if (Sync)
+      Opts.AsyncThreadsCount = 0;
+    TUScheduler S(CDB, Opts);
+    std::atomic<int> Counter(0);
+    S.run("add 1", /*Path=*/"", [&] { ++Counter; });
+    S.run("add 2", /*Path=*/"", [&] { Counter += 2; });
+    ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+    EXPECT_EQ(Counter.load(), 3);
+
+    Notification TaskRun;
+    Key<int> TestKey;
+    WithContextValue CtxWithKey(TestKey, 10);
+    const char *Path = "somepath";
+    S.run("props context", Path, [&] {
+      EXPECT_EQ(Context::current().getExisting(TestKey), 10);
+      EXPECT_EQ(Path, boundPath());
+      TaskRun.notify();
+    });
+    TaskRun.wait();
+  }
 }
 
 TEST_F(TUSchedulerTests, TUStatus) {